From: dan Date: Tue, 5 May 2020 20:30:07 +0000 (+0000) Subject: Unless upgrading an existing read transaction, have ENABLE_SETLK_TIMEOUT builds attem... X-Git-Tag: version-3.32.0~38^2~11 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=58021b237ae7e84069aaa564c0b6834a23296aed;p=thirdparty%2Fsqlite.git Unless upgrading an existing read transaction, have ENABLE_SETLK_TIMEOUT builds attempt to use a blocking lock when opening a write transaction on a wal mode database. FossilOrigin-Name: d6f819a9e6b35f3fd558bd93255a6a24ad690a0fa15a82b009ca9c641db983c6 --- diff --git a/manifest b/manifest index fdafb00a6b..8d829988c3 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Changes\sto\savoid\sdeadlock\sin\sSQLITE_ENABLE_SETLK_TIMEOUT\sbuilds. -D 2020-05-04T19:42:35.009 +C Unless\supgrading\san\sexisting\sread\stransaction,\shave\sENABLE_SETLK_TIMEOUT\sbuilds\sattempt\sto\suse\sa\sblocking\slock\swhen\sopening\sa\swrite\stransaction\son\sa\swal\smode\sdatabase. +D 2020-05-05T20:30:07.254 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -474,7 +474,7 @@ F src/auth.c a3d5bfdba83d25abed1013a8c7a5f204e2e29b0c25242a56bc02bb0c07bf1e06 F src/backup.c 5e617c087f1c2d6005c2ec694ce80d6e16bc68d906e1b1c556d7c7c2228b636b F src/bitvec.c 17ea48eff8ba979f1f5b04cc484c7bb2be632f33 F src/btmutex.c 8acc2f464ee76324bf13310df5692a262b801808984c1b79defb2503bbafadb6 -F src/btree.c 54d404ff88f1432fc056c638e4868fe66dac98cb0f89a6cae4bd6b636053af0f +F src/btree.c 2a152165efcbde2a0df808705cb44b8d2588440f0a9ebcbe534ca95843c10355 F src/btree.h 989ef3c33413549e3e148f3dcb46c030f317dac130dc86809ba6b9aa4b16c72a F src/btreeInt.h 887cdd2ea7f4a65143074a8a7c8928b0546f8c18dda3c06a408ce7992cbab0c0 F src/build.c ec6c0bda1e43ef55e5f5121a77ba19fac51fc6585f95ce2da795bcedcf6e8f36 @@ -517,11 +517,11 @@ F src/os.c 669cc3839cc35d20f81faf0be1ab6d4581cea35e9d8f3a9d48a98d6571f7c285 F src/os.h 48388821692e87da174ea198bf96b1b2d9d83be5dfc908f673ee21fafbe0d432 F src/os_common.h b2f4707a603e36811d9b1a13278bffd757857b85 F src/os_setup.h 0dbaea40a7d36bf311613d31342e0b99e2536586 -F src/os_unix.c 7ef8b60222558a373d89c18d0c3bc44365b45273a528183d40bec5bb76ce23fc +F src/os_unix.c daabdb3c33ef4a6b49ff6809968af2f8c0d3c5811bc654d74375f015180235aa F src/os_win.c 035a813cbd17f355bdcad7ab894af214a9c13a1db8aeac902365350b98cd45a7 F src/os_win.h 7b073010f1451abe501be30d12f6bc599824944a -F src/pager.c 8b5a3b3d12706a8692bf9b36ef0fb94029d855730f51e4615afddb9c7cc4b301 -F src/pager.h eaf8bd9b78f5033a480f66d3a9b5426d04ee352586ee75b9bc7cd50670f6a5b1 +F src/pager.c be4d6d58df5c5d02b84547fd253a80f035ea2ee53931a5393d96af0ba3f04edb +F src/pager.h 5e2c0e5bd32aa4ae6c25eda2722490bb3f671f24d2e5d665d33df13285c1d520 F src/parse.y c8eff38606f443d5ba245263fa7abc05e4116d95656e050c4b78e9bfbf931add F src/pcache.c 385ff064bca69789d199a98e2169445dc16e4291fa807babd61d4890c3b34177 F src/pcache.h 4f87acd914cef5016fae3030343540d75f5b85a1877eed1a2a19b9f284248586 @@ -617,8 +617,8 @@ F src/vdbetrace.c fa3bf238002f0bbbdfb66cc8afb0cea284ff9f148d6439bc1f6f2b4c3b7143 F src/vdbevtab.c 8094dfc28dad82d60a1c832020a1b201a5381dc185c14638affc6d4e9d54c653 F src/vtab.c 7b704a90515a239c6cdba6a66b1bb3a385e62326cceb5ecb05ec7a091d6b8515 F src/vxworks.h d2988f4e5a61a4dfe82c6524dd3d6e4f2ce3cdb9 -F src/wal.c b6c5fb152c9ed73017202b4ffda1ee97a8c6cc57a3596e29ab4b763207013d49 -F src/wal.h 642f7896526181a95dde213fe1a0a515899d8eb2fa7cb562f042b0637d11d3d7 +F src/wal.c 4099109dcf2b302e0d151e6fa0678d9bbc3eb923b8bd13a3cd07db0e7df5016b +F src/wal.h 484890eaaefb89c0f2043ae34c20bf367383f201f63da631ecae424797a7f60d F src/walker.c 7c429c694abd12413a5c17aec9f47cfe9eba6807e6b0a32df883e8e3a14835ed F src/where.c 9546c82056e8cdb27291f98cf1adca5d271240b399bb97b32f77fc2bea6146c9 F src/whereInt.h 6b874aa15f94e43a2cec1080be64d955b04deeafeac90ffb5d6975c0d511be3c @@ -1658,7 +1658,7 @@ F test/vtab_alter.test 736e66fb5ec7b4fee58229aa3ada2f27ec58bc58c00edae4836890c37 F test/vtab_err.test dcc8b7b9cb67522b3fe7a272c73856829dae4ab7fdb30399aea1b6981bda2b65 F test/vtab_shared.test 5253bff2355a9a3f014c15337da7e177ab0ef8ad F test/vtabdrop.test 65d4cf6722972e5499bdaf0c0d70ee3b8133944a4e4bc31862563f32a7edca12 -F test/wal.test cdf0ca6cc0447520d19ef1c83287824ebeb3e82d75af856511ba96841a79fc9b +F test/wal.test 16180bc4becda176428ad02eaea437b4b8f5ae099314de443a4e12b2dcc007a2 F test/wal2.test 537f59e5c5932e3b45bf3591ae3e48a2601360c2e52821b633e222fe6ebd5b09 F test/wal3.test 2a93004bc0fb2b5c29888964024695bade278ab2 F test/wal4.test 4744e155cd6299c6bd99d3eab1c82f77db9cdb3c @@ -1863,10 +1863,7 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P d1ba026d882f070b351280028e2fa88a3cca59b46d4683302e24c6677e0951b9 -R 7034fe62283804a1df8fc178d6719787 -T *branch * setlk-deadlock-changes -T *sym-setlk-deadlock-changes * -T -sym-trunk * +P 553423c23142cf0ec219192315d57ce8a0e10c3d8678d28bc110a1a9a7c17cee +R aa33971255c5eac84a2aa86cb20ae159 U dan -Z b3c62bfe3502546b0ce8fe477018d01b +Z dc53b638850c8fa914d5b99111645e25 diff --git a/manifest.uuid b/manifest.uuid index fdb4fc8030..2f292c1a11 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -553423c23142cf0ec219192315d57ce8a0e10c3d8678d28bc110a1a9a7c17cee \ No newline at end of file +d6f819a9e6b35f3fd558bd93255a6a24ad690a0fa15a82b009ca9c641db983c6 \ No newline at end of file diff --git a/src/btree.c b/src/btree.c index 76845e072a..3b8d0529ce 100644 --- a/src/btree.c +++ b/src/btree.c @@ -3437,6 +3437,18 @@ int sqlite3BtreeBeginTrans(Btree *p, int wrflag, int *pSchemaVersion){ pBt->btsFlags &= ~BTS_INITIALLY_EMPTY; if( pBt->nPage==0 ) pBt->btsFlags |= BTS_INITIALLY_EMPTY; do { + Pager *pPager = pBt->pPager; + +#ifdef SQLITE_ENABLE_SETLK_TIMEOUT + /* If transitioning from no transaction directly to a write transaction, + ** block for the WRITER lock first if possible. */ + if( pBt->pPage1==0 && wrflag ){ + assert( pBt->inTransaction==TRANS_NONE ); + rc = sqlite3PagerWalWriteLock(p->db, pPager, 1); + if( rc!=SQLITE_OK ) break; + } +#endif + /* Call lockBtree() until either pBt->pPage1 is populated or ** lockBtree() returns something other than SQLITE_OK. lockBtree() ** may return SQLITE_OK but leave pBt->pPage1 set to 0 if after @@ -3450,7 +3462,7 @@ int sqlite3BtreeBeginTrans(Btree *p, int wrflag, int *pSchemaVersion){ if( (pBt->btsFlags & BTS_READ_ONLY)!=0 ){ rc = SQLITE_READONLY; }else{ - rc = sqlite3PagerBegin(pBt->pPager,wrflag>1,sqlite3TempInMemory(p->db)); + rc = sqlite3PagerBegin(pPager, wrflag>1, sqlite3TempInMemory(p->db)); if( rc==SQLITE_OK ){ rc = newDatabase(pBt); }else if( rc==SQLITE_BUSY_SNAPSHOT && pBt->inTransaction==TRANS_NONE ){ @@ -3463,6 +3475,7 @@ int sqlite3BtreeBeginTrans(Btree *p, int wrflag, int *pSchemaVersion){ } if( rc!=SQLITE_OK ){ + sqlite3PagerWalWriteLock(p->db, pPager, 0); unlockBtreeIfUnused(pBt); } }while( (rc&0xFF)==SQLITE_BUSY && pBt->inTransaction==TRANS_NONE && diff --git a/src/os_unix.c b/src/os_unix.c index 56e53929ea..e1ed575b01 100644 --- a/src/os_unix.c +++ b/src/os_unix.c @@ -4819,22 +4819,24 @@ static int unixShmLock( assert( pShmNode->hShm>=0 || pDbFd->pInode->bProcessLock==1 ); assert( pShmNode->hShm<0 || pDbFd->pInode->bProcessLock==0 ); - /* Check that, if this to be a blocking lock, that locks have been - ** obtained in the following order. + /* Check that, if this to be a blocking lock, no locks that occur later + ** in the following list than the lock being obtained are already held: ** ** 1. Checkpointer lock (ofst==1). - ** 2. Recover lock (ofst==2). + ** 2. Write lock (ofst==0). ** 3. Read locks (ofst>=3 && ofstiBusyTimeout==0 - || (flags & SQLITE_SHM_UNLOCK) || ofst==0 - || ((p->exclMask|p->sharedMask)&~((1<iBusyTimeout==0 || ( + (ofst!=2) /* not RECOVER */ + && (n==1) /* Single lock only */ + && (ofst!=1 || (p->exclMask|p->sharedMask)==0) + && (ofst!=0 || (p->exclMask|p->sharedMask)<3) + && (ofst<3 || (p->exclMask|p->sharedMask)<(1<exclusiveMode==0 ){ + rc = sqlite3WalWriteLock(db, pPager->pWal, bLock); + } + return rc; +} +#endif #ifdef SQLITE_ENABLE_SNAPSHOT /* diff --git a/src/pager.h b/src/pager.h index a044daa18b..c203908c83 100644 --- a/src/pager.h +++ b/src/pager.h @@ -185,6 +185,12 @@ int sqlite3PagerSharedLock(Pager *pPager); # endif #endif +#if !defined(SQLITE_OMIT_WAL) && defined(SQLITE_ENABLE_SETLK_TIMEOUT) + int sqlite3PagerWalWriteLock(sqlite3*, Pager*, int); +#else +# define sqlite3PagerWalWriteLock(x,y,z) SQLITE_OK +#endif + #ifdef SQLITE_DIRECT_OVERFLOW_READ int sqlite3PagerDirectReadOk(Pager *pPager, Pgno pgno); #endif diff --git a/src/wal.c b/src/wal.c index 020a481e28..a70a716244 100644 --- a/src/wal.c +++ b/src/wal.c @@ -2731,6 +2731,65 @@ int sqlite3WalSnapshotRecover(Wal *pWal){ } #endif /* SQLITE_ENABLE_SNAPSHOT */ +#ifdef SQLITE_ENABLE_SETLK_TIMEOUT +/* +** Attempt to enable blocking locks. Blocking locks are enabled only if (a) +** they are supported by the VFS, and (b) the database handle is configured +** with a busy-timeout. Return 1 if blocking locks are successfully enabled, +** or 0 otherwise. +*/ +static int walEnableBlocking(sqlite3 *db, Wal *pWal){ + int res = 0; + if( db->busyTimeout ){ + int rc; + int tmout = db->busyTimeout; + rc = sqlite3OsFileControl( + pWal->pDbFd, SQLITE_FCNTL_LOCK_TIMEOUT, (void*)&tmout + ); + res = (rc==SQLITE_OK); + } + return res; +} + +/* +** Disable blocking locks. +*/ +static void walDisableBlocking(Wal *pWal){ + int tmout = 0; + sqlite3OsFileControl(pWal->pDbFd, SQLITE_FCNTL_LOCK_TIMEOUT, (void*)&tmout); +} + +/* +** If parameter bLock is true, attempt to enable blocking locks, take +** the WRITER lock, and then disable blocking locks. If blocking locks +** cannot be enabled, no attempt to obtain the WRITER lock is made. Return +** an SQLite error code if an error occurs, or SQLITE_OK otherwise. It is not +** an error if blocking locks can not be enabled. +** +** If the bLock parameter is false and the WRITER lock is held, release it. +*/ +int sqlite3WalWriteLock(sqlite3 *db, Wal *pWal, int bLock){ + int rc = SQLITE_OK; + assert( pWal->readLock<0 || bLock==0 ); + if( bLock ){ + if( walEnableBlocking(db, pWal) ){ + rc = walLockExclusive(pWal, WAL_WRITE_LOCK, 1); + if( rc==SQLITE_OK ){ + pWal->writeLock = 1; + } + walDisableBlocking(pWal); + } + }else if( pWal->writeLock ){ + walUnlockExclusive(pWal, WAL_WRITE_LOCK, 1); + pWal->writeLock = 0; + } + return rc; +} +#else +# define walEnableBlocking(x,y) 0 +# define walDisableBlocking(x) +#endif /* ifdef SQLITE_ENABLE_SETLK_TIMEOUT */ + /* ** Begin a read transaction on the database. ** @@ -2754,14 +2813,6 @@ int sqlite3WalBeginReadTransaction(Wal *pWal, int *pChanged){ int bChanged = 0; WalIndexHdr *pSnapshot = pWal->pSnapshot; if( pSnapshot ){ -#ifdef SQLITE_ENABLE_SETLK_TIMEOUT - int busyTimeout = pWal->dbSnapshot->busyTimeout; - if( busyTimeout ){ - int tmout = busyTimeout; - sqlite3OsFileControl(pWal->pDbFd,SQLITE_FCNTL_LOCK_TIMEOUT,(void*)&tmout); - } -#endif - if( memcmp(pSnapshot, &pWal->hdr, sizeof(WalIndexHdr))!=0 ){ bChanged = 1; } @@ -2774,14 +2825,9 @@ int sqlite3WalBeginReadTransaction(Wal *pWal, int *pChanged){ ** its intent. To avoid the race condition this leads to, ensure that ** there is no checkpointer process by taking a shared CKPT lock ** before checking pInfo->nBackfillAttempted. */ + walEnableBlocking(pWal->dbSnapshot, pWal); rc = walLockShared(pWal, WAL_CKPT_LOCK); - -#ifdef SQLITE_ENABLE_SETLK_TIMEOUT - if( busyTimeout ){ - int tmout = 0; - sqlite3OsFileControl(pWal->pDbFd,SQLITE_FCNTL_LOCK_TIMEOUT,(void*)&tmout); - } -#endif + walDisableBlocking(pWal); if( rc!=SQLITE_OK ){ return rc; @@ -2861,8 +2907,8 @@ int sqlite3WalBeginReadTransaction(Wal *pWal, int *pChanged){ ** read-lock. */ void sqlite3WalEndReadTransaction(Wal *pWal){ - sqlite3WalEndWriteTransaction(pWal); if( pWal->readLock>=0 ){ + sqlite3WalEndWriteTransaction(pWal); walUnlockShared(pWal, WAL_READ_LOCK(pWal->readLock)); pWal->readLock = -1; } @@ -3022,6 +3068,16 @@ Pgno sqlite3WalDbsize(Wal *pWal){ int sqlite3WalBeginWriteTransaction(Wal *pWal){ int rc; +#ifdef SQLITE_ENABLE_SETLK_TIMEOUT + /* If the write-lock is already held, then it was obtained before the + ** read-transaction was even opened, making this call a no-op. + ** Return early. */ + if( pWal->writeLock ){ + assert( !memcmp(&pWal->hdr,(void *)walIndexHdr(pWal),sizeof(WalIndexHdr)) ); + return SQLITE_OK; + } +#endif + /* Cannot start a write transaction without first holding a read ** transaction. */ assert( pWal->readLock>=0 ); @@ -3587,9 +3643,6 @@ int sqlite3WalCheckpoint( int isChanged = 0; /* True if a new wal-index header is loaded */ int eMode2 = eMode; /* Mode to pass to walCheckpoint() */ int (*xBusy2)(void*) = xBusy; /* Busy handler for eMode2 */ -#ifdef SQLITE_ENABLE_SETLK_TIMEOUT - int bSetLk = 0; -#endif assert( pWal->ckptLock==0 ); assert( pWal->writeLock==0 ); @@ -3601,18 +3654,11 @@ int sqlite3WalCheckpoint( if( pWal->readOnly ) return SQLITE_READONLY; WALTRACE(("WAL%p: checkpoint begins\n", pWal)); -#ifdef SQLITE_ENABLE_SETLK_TIMEOUT - if( db->busyTimeout ){ - int tmout = db->busyTimeout; - sqlite3_file *fd = pWal->pDbFd; - if( SQLITE_OK== - sqlite3OsFileControl(fd, SQLITE_FCNTL_LOCK_TIMEOUT, (void*)&tmout) - ){ - xBusy2 = 0; - bSetLk = 1; - } + /* Enable blocking locks, if possible. If blocking locks are successfully + ** enabled, set xBusy2=0 so that the busy-handler is never invoked. */ + if( walEnableBlocking(db, pWal) ){ + xBusy2 = 0; } -#endif /* IMPLEMENTATION-OF: R-62028-47212 All calls obtain an exclusive ** "checkpoint" lock on the database file. @@ -3684,13 +3730,7 @@ int sqlite3WalCheckpoint( memset(&pWal->hdr, 0, sizeof(WalIndexHdr)); } -#ifdef SQLITE_ENABLE_SETLK_TIMEOUT - if( bSetLk ){ - int tmout = 0; - sqlite3_file *fd = pWal->pDbFd; - sqlite3OsFileControl(fd, SQLITE_FCNTL_LOCK_TIMEOUT, (void*)&tmout); - } -#endif + walDisableBlocking(pWal); /* Release the locks. */ sqlite3WalEndWriteTransaction(pWal); diff --git a/src/wal.h b/src/wal.h index bebd643700..99ebc937af 100644 --- a/src/wal.h +++ b/src/wal.h @@ -146,5 +146,9 @@ int sqlite3WalFramesize(Wal *pWal); /* Return the sqlite3_file object for the WAL file */ sqlite3_file *sqlite3WalFile(Wal *pWal); +#ifdef SQLITE_ENABLE_SETLK_TIMEOUT +int sqlite3WalWriteLock(sqlite3 *db, Wal *pWal, int bLock); +#endif + #endif /* ifndef SQLITE_OMIT_WAL */ #endif /* SQLITE_WAL_H */ diff --git a/test/wal.test b/test/wal.test index a003b6ad20..acc2780081 100644 --- a/test/wal.test +++ b/test/wal.test @@ -43,6 +43,7 @@ proc sqlite3_wal {args} { [lindex $args 0] eval { PRAGMA journal_mode = wal } [lindex $args 0] eval { PRAGMA synchronous = normal } [lindex $args 0] function blob blob + db timeout 1000 } proc log_deleted {logfile} {