Ensure that SQLITE_PROTOCOL is not returned too early when a SQLITE_ENABLE_SETLK_TIMEOUT build fails to open a transaction on a wal mode database in cases where blocking locks are not being used.

FossilOrigin-Name: a8e9af1356f5fb2ec460f932dfbe89283bb4e3cf9fa677d1acdbe77ffa11dd04

Author: dan

manifest

@@ -1,5 +1,5 @@
-C Remove\san\sALWAYS()\sfrom\sRTREE.\s\sDbsqlfuzz\sfound\sa\sway\sto\smake\sit\sfalse.
-D 2024-01-07T20:27:54.639
+C Ensure\sthat\sSQLITE_PROTOCOL\sis\snot\sreturned\stoo\searly\swhen\sa\sSQLITE_ENABLE_SETLK_TIMEOUT\sbuild\sfails\sto\sopen\sa\stransaction\son\sa\swal\smode\sdatabase\sin\scases\swhere\sblocking\slocks\sare\snot\sbeing\sused.
+D 2024-01-08T13:38:15.023
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -819,7 +819,7 @@ F src/vdbetrace.c fe0bc29ebd4e02c8bc5c1945f1d2e6be5927ec12c06d89b03ef2a4def34bf8
 F src/vdbevtab.c 2143db7db0ceed69b21422581f434baffc507a08d831565193a7a02882a1b6a7
 F src/vtab.c 11948e105f56e84099ca17f1f434b1944539ea84de26d0d767eadfbc670ce1ea
 F src/vxworks.h d2988f4e5a61a4dfe82c6524dd3d6e4f2ce3cdb9
-F src/wal.c b08f88e69b9c43572f0703355dccc5b2f08296256fc6a82a582fcae68f8f9cd1
+F src/wal.c bf41478461ca8a2758617691152befd9d526921e2fb59d5647638679cff98381
 F src/wal.h ba252daaa94f889f4b2c17c027e823d9be47ce39da1d3799886bbd51f0490452
 F src/walker.c 7c7ea0115345851c3da4e04e2e239a29983b61fb5b038b94eede6aba462640e2
 F src/where.c 217fe82a26c0fb6a3c7fd01865d821e752f9c01fb72f114af3f0b77ce234d1fb
@@ -2156,8 +2156,9 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P 7a5b42ff74882c58493dc8b710fde73d4ff251f5d42271d84be73ceaabc01698
-R e24ddedf31de297bbf4b76d307641631
-U drh
-Z ac5f82bfe64ed7224d42f062673e1893
+P 40f0a29e6dd90fcb969d7c0e49728ba0ee8f31d9e8f502b9a21469620a8ad283 b934a33671d8a0190082ad7e5e68c78fe0c558d102404eafc1de26e4e7d65b92
+R faa7ffdec00af93a5c93f6a8d8d1ed79
+T +closed b934a33671d8a0190082ad7e5e68c78fe0c558d102404eafc1de26e4e7d65b92
+U dan
+Z f6fdb69739d322cda88ea774ad465280
 # Remove this line to create a well-formed Fossil manifest.

manifest.uuid

@@ -1 +1 @@
-40f0a29e6dd90fcb969d7c0e49728ba0ee8f31d9e8f502b9a21469620a8ad283
+a8e9af1356f5fb2ec460f932dfbe89283bb4e3cf9fa677d1acdbe77ffa11dd04

src/wal.c

@@ -2902,6 +2902,37 @@ static int walBeginShmUnreliable(Wal *pWal, int *pChanged){
   return rc;
 }
 
+/*
+** The final argument passed to walTryBeginRead() is of type (int*). The
+** caller should invoke walTryBeginRead as follows:
+**
+**   int cnt = 0;
+**   do {
+**     rc = walTryBeginRead(..., &cnt);
+**   }while( rc==WAL_RETRY );
+**
+** The final value of "cnt" is of no use to the caller. It is used by
+** the implementation of walTryBeginRead() as follows:
+**
+**   + Each time walTryBeginRead() is called, it is incremented. Once
+**     it reaches WAL_RETRY_PROTOCOL_LIMIT - indicating that walTryBeginRead()
+**     has many times been invoked and failed with WAL_RETRY - walTryBeginRead()
+**     returns SQLITE_PROTOCOL.
+**
+**   + If SQLITE_ENABLE_SETLK_TIMEOUT is defined and walTryBeginRead() failed
+**     because a blocking lock timed out (SQLITE_BUSY_TIMEOUT from the OS
+**     layer), the WAL_RETRY_BLOCKED_MASK bit is set in "cnt". In this case
+**     the next invocation of walTryBeginRead() may omit an expected call to 
+**     sqlite3OsSleep(). There has already been a delay when the previous call
+**     waited on a lock.
+*/
+#define WAL_RETRY_PROTOCOL_LIMIT 100
+#ifdef SQLITE_ENABLE_SETLK_TIMEOUT
+# define WAL_RETRY_BLOCKED_MASK    0x10000000
+#else
+# define WAL_RETRY_BLOCKED_MASK    0
+#endif
+
 /*
 ** Attempt to start a read transaction.  This might fail due to a race or
 ** other transient condition.  When that happens, it returns WAL_RETRY to
@@ -2952,7 +2983,7 @@ static int walBeginShmUnreliable(Wal *pWal, int *pChanged){
 ** so it takes care to hold an exclusive lock on the corresponding
 ** WAL_READ_LOCK() while changing values.
 */
-static int walTryBeginRead(Wal *pWal, int *pChanged, int useWal, int cnt){
+static int walTryBeginRead(Wal *pWal, int *pChanged, int useWal, int *pCnt){
   volatile WalCkptInfo *pInfo;    /* Checkpoint information in wal-index */
   u32 mxReadMark;                 /* Largest aReadMark[] value */
   int mxI;                        /* Index of largest aReadMark[] value */
@@ -2985,27 +3016,34 @@ static int walTryBeginRead(Wal *pWal, int *pChanged, int useWal, int cnt){
   ** so that on the 100th (and last) RETRY we delay for 323 milliseconds.
   ** The total delay time before giving up is less than 10 seconds.
   */
-  if( cnt>5 ){
+  (*pCnt)++;
+  if( *pCnt>5 ){
     int nDelay = 1;                      /* Pause time in microseconds */
-    if( cnt>100 ){
+    int cnt = (*pCnt & ~WAL_RETRY_BLOCKED_MASK);
+    if( cnt>WAL_RETRY_PROTOCOL_LIMIT ){
       VVA_ONLY( pWal->lockError = 1; )
       return SQLITE_PROTOCOL;
     }
-    if( cnt>=10 ) nDelay = (cnt-9)*(cnt-9)*39;
+    if( *pCnt>=10 ) nDelay = (cnt-9)*(cnt-9)*39;
 #ifdef SQLITE_ENABLE_SETLK_TIMEOUT
     /* In SQLITE_ENABLE_SETLK_TIMEOUT builds, configure the file-descriptor
     ** to block for locks for approximately nDelay us. This affects three
     ** locks: (a) the shared lock taken on the DMS slot in os_unix.c (if
     ** using os_unix.c), (b) the WRITER lock taken in walIndexReadHdr() if the
-    ** first attempted read fails, and (c) the shared lock taken on the DMS
-    ** slot in os_unix.c. All three of these locks are attempted from within 
-    ** the call to walIndexReadHdr() below.  */
+    ** first attempted read fails, and (c) the shared lock taken on the 
+    ** read-mark.  
+    **
+    ** If the previous call failed due to an SQLITE_BUSY_TIMEOUT error,
+    ** then sleep for the minimum of 1us. The previous call already provided 
+    ** an extra delay while it was blocking on the lock.
+    */
     nBlockTmout = (nDelay+998) / 1000;
     if( !useWal && walEnableBlockingMs(pWal, nBlockTmout) ){
-      nDelay = 1;
+      if( *pCnt & WAL_RETRY_BLOCKED_MASK ) nDelay = 1;
     }
 #endif
     sqlite3OsSleep(pWal->pVfs, nDelay);
+    *pCnt &= ~WAL_RETRY_BLOCKED_MASK;
   }
 
   if( !useWal ){
@@ -3015,7 +3053,10 @@ static int walTryBeginRead(Wal *pWal, int *pChanged, int useWal, int cnt){
     }
 #ifdef SQLITE_ENABLE_SETLK_TIMEOUT
     walDisableBlocking(pWal);
-    if( rc==SQLITE_BUSY_TIMEOUT ) rc = SQLITE_BUSY;
+    if( rc==SQLITE_BUSY_TIMEOUT ){
+      rc = SQLITE_BUSY;
+      *pCnt |= WAL_RETRY_BLOCKED_MASK;
+    }
 #endif
     if( rc==SQLITE_BUSY ){
       /* If there is not a recovery running in another thread or process
@@ -3135,6 +3176,9 @@ static int walTryBeginRead(Wal *pWal, int *pChanged, int useWal, int cnt){
   rc = walLockShared(pWal, WAL_READ_LOCK(mxI));
   walDisableBlocking(pWal);
   if( rc ){
+    if( rc==SQLITE_BUSY_TIMEOUT ){
+      *pCnt |= WAL_RETRY_BLOCKED_MASK;
+    }
     assert( (rc&0xFF)!=SQLITE_BUSY||rc==SQLITE_BUSY||rc==SQLITE_BUSY_TIMEOUT );
     return (rc&0xFF)==SQLITE_BUSY ? WAL_RETRY : rc;
   }
@@ -3324,7 +3368,7 @@ static int walBeginReadTransaction(Wal *pWal, int *pChanged){
 #endif
 
   do{
-    rc = walTryBeginRead(pWal, pChanged, 0, ++cnt);
+    rc = walTryBeginRead(pWal, pChanged, 0, &cnt);
   }while( rc==WAL_RETRY );
   testcase( (rc&0xff)==SQLITE_BUSY );
   testcase( (rc&0xff)==SQLITE_IOERR );
@@ -3809,7 +3853,7 @@ static int walRestartLog(Wal *pWal){
     cnt = 0;
     do{
       int notUsed;
-      rc = walTryBeginRead(pWal, &notUsed, 1, ++cnt);
+      rc = walTryBeginRead(pWal, &notUsed, 1, &cnt);
     }while( rc==WAL_RETRY );
     assert( (rc&0xff)!=SQLITE_BUSY ); /* BUSY not possible when useWal==1 */
     testcase( (rc&0xff)==SQLITE_IOERR );