Unless upgrading an existing read transaction, have ENABLE_SETLK_TIMEOUT builds attem...

FossilOrigin-Name: d6f819a9e6b35f3fd558bd93255a6a24ad690a0fa15a82b009ca9c641db983c6

diff --git a/manifest b/manifest
index fdafb00a6bf7deb4ed6dc1020e2bfb4a50d74745..8d829988c381b1841258fa8fc8b227981b125798 100644 (file)
--- 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 fdb4fc80307bbe8de2458bb4228dab55bc0ae3de..2f292c1a1181cf362680c5c60cee6bf9cf78741d 100644 (file)
--- 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 76845e072af638d83b7ef5f696b29fbee9b36b83..3b8d0529ced9b1eac5898e4df1af8b94c479604b 100644 (file)
--- 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 56e53929ea012f5c14aafb14d941e84c8373e66d..e1ed575b01478ebd1dc9d6b9c3fced32509dd470 100644 (file)
--- 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 && ofst<SQLITE_SHM_NLOCK).
-  **   4. Write lock (ofst==0).
   **
   ** In other words, if this is a blocking lock, none of the locks that
   ** occur later in the above list than the lock being obtained may be
   ** held.  */
 #ifdef SQLITE_ENABLE_SETLK_TIMEOUT
-  assert( pDbFd->iBusyTimeout==0 
-       || (flags & SQLITE_SHM_UNLOCK) || ofst==0
-       || ((p->exclMask|p->sharedMask)&~((1<<ofst)-2))==0
-  );
+  assert( (flags & SQLITE_SHM_UNLOCK) || pDbFd->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<<ofst))
+  ));
 #endif
 
   mask = (1<<(ofst+n)) - (1<<ofst);

diff --git a/src/pager.c b/src/pager.c
index dd723b788d186e6a196ac8ab381b57b516a8b93f..1570b3e704580bca48b0bf6e84a28facba7962a8 100644 (file)
--- a/src/pager.c
+++ b/src/pager.c
@@ -7574,7 +7574,21 @@ int sqlite3PagerCloseWal(Pager *pPager, sqlite3 *db){
   return rc;
 }
 
-
+#ifdef SQLITE_ENABLE_SETLK_TIMEOUT
+/*
+** If pager pPager is a wal-mode database not in exclusive locking mode,
+** invoke the sqlite3WalWriteLock() function on the associated Wal object 
+** with the same db and bLock parameters as were passed to this function.
+** Return an SQLite error code if an error occurs, or SQLITE_OK otherwise.
+*/
+int sqlite3PagerWalWriteLock(sqlite3 *db, Pager *pPager, int bLock){
+  int rc = SQLITE_OK;
+  if( pagerUseWal(pPager) && pPager->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 a044daa18b599613f94dc39d5315b043b6af0e9c..c203908c83b9141865fd364a63a9136a07aa7d59 100644 (file)
--- 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 020a481e2801933601d82f2f7f0cfcd18c671952..a70a71624432bb588ab3c868f1c385498e88747c 100644 (file)
--- 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 bebd643700d4344c250a6bd4587d3c616cffff5f..99ebc937af19650592c9dca3d9c9664771fd32f7 100644 (file)
--- 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 a003b6ad20abd62c443d49512682e26a5bc17622..acc278008163ff274334c8fb7e8aad82f6de37e5 100644 (file)
--- 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} {