From 4e50f7763bb01c2b9a570f8679d9f2e32ba1b51e Mon Sep 17 00:00:00 2001 From: dan Date: Sat, 6 Jan 2024 19:16:54 +0000 Subject: [PATCH] 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: b934a33671d8a0190082ad7e5e68c78fe0c558d102404eafc1de26e4e7d65b92 --- manifest | 16 +++++++------ manifest.uuid | 2 +- src/wal.c | 66 ++++++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 65 insertions(+), 19 deletions(-) diff --git a/manifest b/manifest index f279291b3e..3807576f53 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Update\sextension\sext/misc/totext.c\sto\savoid\sboth\subsan\swarnings\sand\sdubious\sreal->integer\sconversions. -D 2024-01-06T13:58:54.947 +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-06T19:16:54.624 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,9 +2156,11 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P 8940e2a1054fbc19fae3f76e743d744840c3a5aad001be8d3d56ca134226c34b c626aa108a7a30cef54af8d93ac9e45749568ed38e4e06623a6bad6b4bf6e8ec -R a29b7304699ca19c7db12a157b970ee6 -T +closed c626aa108a7a30cef54af8d93ac9e45749568ed38e4e06623a6bad6b4bf6e8ec +P 541436004df6458dc2f38bdfa421099f78cab397f5f4795ca5f227531aaa6a3e +R 93c12d3967c4e74bf7f57f8f4eca1454 +T *branch * enable-setlk-fix +T *sym-enable-setlk-fix * +T -sym-trunk * U dan -Z 28d29fe86f310229597cb641cf92c44b +Z 21f53e2f4fe1872998898c66db1dee4c # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 87dc330212..770ea46d73 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -541436004df6458dc2f38bdfa421099f78cab397f5f4795ca5f227531aaa6a3e \ No newline at end of file +b934a33671d8a0190082ad7e5e68c78fe0c558d102404eafc1de26e4e7d65b92 \ No newline at end of file diff --git a/src/wal.c b/src/wal.c index eb97831dcb..4e49e707b3 100644 --- a/src/wal.c +++ b/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, ¬Used, 1, ++cnt); + rc = walTryBeginRead(pWal, ¬Used, 1, &cnt); }while( rc==WAL_RETRY ); assert( (rc&0xff)!=SQLITE_BUSY ); /* BUSY not possible when useWal==1 */ testcase( (rc&0xff)==SQLITE_IOERR ); -- 2.39.5