Those retry loops are there to work around virus checkers, which can prevent calls to winAccess() and similar from succeeding until the virus checker has finished its scan of the file. So if we turn the retry loop off, or if we reduce the timeout, then then we get lots of failures caused by virus checking.

What is your suggested fix? How do we delay these operations when they are blocked by an active virus checker, but not delay them when the file simply does not exist? How do we tell the difference?

I understand the reason for the retry loop around CreateFile. I'm still figuring out the point of testing read-access when trying to open the database readwrite. I've found a commit message that says:

In the Windows VFS, when trying to open a database file read/write, if it fails check to see if the file exists and is read-only and immediately fall back to a read-only open attempt, rather than running the AV retry loop.

I'm convinced that one retryloop should not be nested in another. Since the check looks like it was inteded to be an optimization, then maybe it would be better to call it only on the first iteration?

Can you please try checkin 2025-03-26t14:45Z and report back whether or not this clears you problem?

Thanks for the patch. I've tried a similar approach in the meantime, but without the NORETRY option, I've simply added the cnt==0 condition.

Here are the results:

• original sqlite: 20 seconds
• with the patch: 6 seconds
• my simpler approach without NORETRY: 7.5 seconds

So this is a definite improvement.

The remaining delay is caused by the fact that when winOpen fails in read-write mode, it makes another attempt in read-only mode. If this could be avoided somehow, it would halve the total delay again, but I expect this would require more extensive changes. Like making both attempts (R/W & R/O) within a single retry-loop.

Anyway, thanks for the quick response.

Can you please try again with checkin 2025-03-26t15:51z and report back if there is any further improvement?

The second patch works as expected - the runtime is down to around 3 seconds.

But I wonder this change it isn't too aggressive. The access check is now done only once (cnt==0 and NORETRY), so it is not resistant to AV interference. If the file is marked read-only, but the first GetFileAttributesExW call fails, it would never be repeated, and the fallback to read-only mode would not be triggered.

I think it would work better without the NORETRY flag, since the read-only test would be done robustly (but still only once). This would slow down the previous scenario (file not accessible at all), but you can't have the cake and eat it...

Yet another patch to try, please: checkin 2025-03-26T17:05Z.

Ok, this one seems to work just as well as the previous one (around 3 seconds), without any obvious drawbacks. Thanks!

I'm doing a catch-up to the latest SQLite and stumbled upon an obscure case where a spurious error log line is generated, due to the changes triggered by this thread.

If the file does not exist and is opened via

sqlite3_open_v2("test.db", &pDb, SQLITE_OPEN_READWRITE, NULL)

specifically using SQLITE_OPEN_READWRITE without SQLITE_OPEN_CREATE, then the winAccess function outputs this in the error log:

os_win.c:53799: (0) winAccess(test.db) - The operation completed successfully.

(There are two expected subsequent errors reporting SQLITE_CANTOPEN from winOpen and a line reference.)

This error log line does not appear in SQLite versions prior to the code changes in this thread.

I've pinpointed why this happens.

As at commit 2c8938f43f721a2592b0efe36f75ab87aa370654

src/os_win.c starting at line 5986

    while( !(rc = osGetFileAttributesExW((LPCWSTR)zConverted,
                             GetFileExInfoStandard,
                             &sAttrData))
       && !noRetry
       && winRetryIoerr(&cnt, &lastErrno)
    ){ /* Loop until true */}

If rc==0 and noRetry!=0 then the AND short-circuits and the call to winRetryIoerr() is skipped. This means lastErrno is not fetched, leaving it set to the initial value of 0. Subsequent code (see below) is checking lastErrno to avoid logging errors for file/path not found, so it logs a "success" error.

I'm not sure about the preferred fix, since swapping the noErr test and winRetryIoerr() call would result in unwanted sleep calls in some cases, and calling osGetLastError() later may fetch the wrong error if there are any other Windows API calls. The following patch is a possible solution.

--- src/os_win.c
+++ src/os_win.c
@@ -5999,10 +5999,13 @@
         attr = INVALID_FILE_ATTRIBUTES;
       }else{
         attr = sAttrData.dwFileAttributes;
       }
     }else{
+      if( noRetry ){
+        lastErrno = osGetLastError();
+      }
       winLogIoerr(cnt, __LINE__);
       if( lastErrno!=ERROR_FILE_NOT_FOUND && lastErrno!=ERROR_PATH_NOT_FOUND ){
         sqlite3_free(zConverted);
         return winLogError(SQLITE_IOERR_ACCESS, lastErrno, "winAccess",
                            zFilename);