From: dan <dan@noemail.net>
Date: Fri, 27 Mar 2020 17:23:17 +0000 (+0000)
Subject: Modifications to the way blocking locks are used in SQLITE_ENABLE_SETLK_TIMEOUT build... 
X-Git-Tag: version-3.32.0~98^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fheads%2Fsetlk-deadlock-fix;p=thirdparty%2Fsqlite.git

Modifications to the way blocking locks are used in SQLITE_ENABLE_SETLK_TIMEOUT builds so that multiple processes or threads may not deadlock when operating on a single database.

FossilOrigin-Name: c516027d5fd876b7d0bf566435667d554db29ded30ad6fc1165caa4a93d015a0
---

diff --git a/manifest b/manifest
index 2e5f6a4e1e..5a774946b5 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Reinstate\sthe\soptimization\sthat\sconverts\s"x\sIN\s(y)"\sinto\s"x==y".
-D 2020-03-26T00:29:50.482
+C Modifications\sto\sthe\sway\sblocking\slocks\sare\sused\sin\sSQLITE_ENABLE_SETLK_TIMEOUT\sbuilds\sso\sthat\smultiple\sprocesses\sor\sthreads\smay\snot\sdeadlock\swhen\soperating\son\sa\ssingle\sdatabase.
+D 2020-03-27T17:23:17.566
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -495,7 +495,7 @@ F src/in-operator.md 10cd8f4bcd225a32518407c2fb2484089112fd71
 F src/insert.c 8e4211d04eb460c0694d486c6ba1c068d468c6f653c3f237869a802ad82854de
 F src/legacy.c d7874bc885906868cd51e6c2156698f2754f02d9eee1bae2d687323c3ca8e5aa
 F src/loadext.c b179df50e6e8bb0c36c149e95d958d49bd8c6c7469e59c01b53d164360bc6c32
-F src/main.c b11eec03807a038b4075d310936dc33ce597078ab78c35dacdf5f05446ed53de
+F src/main.c 0509a4f46c99eb28784d3ee858b436bdaa0d1a5e1f1f67b2d952b9968e9d37c4
 F src/malloc.c eaa4dc9602ce28b077f7de2eb275db2be270c5cc56d7fec5466301bd9b80e2f5
 F src/mem0.c 6a55ebe57c46ca1a7d98da93aaa07f99f1059645
 F src/mem1.c c12a42539b1ba105e3707d0e628ad70e611040d8f5e38cf942cee30c867083de
@@ -515,7 +515,7 @@ 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 238ad16109b4160d0fba2597ef71459995ab5e304b3d8bbe290a8d5d6d6000e0
+F src/os_unix.c 06593ba4bdfae42b30a897b3b4e59abb88a11d50dd0cad39003da955d822b8d1
 F src/os_win.c 035a813cbd17f355bdcad7ab894af214a9c13a1db8aeac902365350b98cd45a7
 F src/os_win.h 7b073010f1451abe501be30d12f6bc599824944a
 F src/pager.c a71ffd145f55e28cbdc1bdabb5e6bef063da428a6c0de3c3a36e9a0c41d4c8c0
@@ -533,7 +533,7 @@ F src/resolve.c 903a70c48d0f72fdd657b225d499cf99ec01d575cf3fbc8196b43562045319ac
 F src/rowset.c ba9515a922af32abe1f7d39406b9d35730ed65efab9443dc5702693b60854c92
 F src/select.c 6908c059e4289149f88bfd641305b96ee34e500855fcd580ff8df1bd0885191a
 F src/shell.c.in 7bb9005bf876c4e1210257a63fa49b556f4eddf59f94b6eb310fcb5096bec0e9
-F src/sqlite.h.in 802957feeb249ede54f8dfe99b72aa19e70a0b7737969c46e625dc2f9f2d42b0
+F src/sqlite.h.in cc7d0949ac32bb68ed97acdb3e7ae91cd413a24d32d6ff049ef8308d620a4367
 F src/sqlite3.rc 5121c9e10c3964d5755191c80dd1180c122fc3a8
 F src/sqlite3ext.h 9c5269260409eb3275324ccace6a13a96f4ad330c708415f70ca6097901ff4ee
 F src/sqliteInt.h 24cc324de1a15b4ab0efdc74277be0b7f90e5f7d29ba2cc2173dea3878974c1b
@@ -614,7 +614,7 @@ F src/vdbesort.c 2be76d26998ce2b3324cdcc9f6443728e54b6c7677c553ad909c7d7cfab587d
 F src/vdbetrace.c fa3bf238002f0bbbdfb66cc8afb0cea284ff9f148d6439bc1f6f2b4c3b7143f0
 F src/vtab.c 7b704a90515a239c6cdba6a66b1bb3a385e62326cceb5ecb05ec7a091d6b8515
 F src/vxworks.h d2988f4e5a61a4dfe82c6524dd3d6e4f2ce3cdb9
-F src/wal.c 697424314e40d99f93f548c7bfa526c10e87f4bdf64d5a76a96b999dd7133ebc
+F src/wal.c b85296a95521d0db188c8a381034f958fab4b3e40f22814f983c4eade6542426
 F src/wal.h 606292549f5a7be50b6227bd685fa76e3a4affad71bb8ac5ce4cb5c79f6a176a
 F src/walker.c 7c429c694abd12413a5c17aec9f47cfe9eba6807e6b0a32df883e8e3a14835ed
 F src/where.c 9546c82056e8cdb27291f98cf1adca5d271240b399bb97b32f77fc2bea6146c9
@@ -1860,7 +1860,10 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P 029ceea612f5c7a1420939d8a772eefcc6f9550d24d57b52ae1729ea9d85fae9
-R 3705b23e9f0d2aa8bb1acee008cd53c5
-U drh
-Z f21f043fe82b450709b51f4ab8aebfa6
+P 27936e6884e77093533719c7955a17f051cfb359872e51a6d1481152e6256443
+R 9bc9d6a926251e677cf43f4b2b436987
+T *branch * setlk-deadlock-fix
+T *sym-setlk-deadlock-fix *
+T -sym-trunk *
+U dan
+Z 8457a3f2f7aaa19e283d3e6cd35c2263
diff --git a/manifest.uuid b/manifest.uuid
index 6d29acdcbb..461b87b10a 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-27936e6884e77093533719c7955a17f051cfb359872e51a6d1481152e6256443
\ No newline at end of file
+c516027d5fd876b7d0bf566435667d554db29ded30ad6fc1165caa4a93d015a0
\ No newline at end of file
diff --git a/src/main.c b/src/main.c
index 98e030fad1..9491eb4b1b 100644
--- a/src/main.c
+++ b/src/main.c
@@ -1569,9 +1569,21 @@ static int sqliteDefaultBusyCallback(
 #ifdef SQLITE_ENABLE_SETLK_TIMEOUT
   if( sqlite3OsFileControl(pFile,SQLITE_FCNTL_LOCK_TIMEOUT,&tmout)==SQLITE_OK ){
     if( count ){
-      tmout = 0;
-      sqlite3OsFileControl(pFile, SQLITE_FCNTL_LOCK_TIMEOUT, &tmout);
-      return 0;
+      /* If this is the second or later invocation of the busy-handler,
+      ** but tmout==0, then code in wal.c must have disabled the blocking
+      ** lock before the SQLITE_BUSY error was hit. In this case, no delay
+      ** occurred while waiting for the lock, so fall through to the xSleep()
+      ** code below to delay a while before retrying the lock.  
+      **
+      ** Alternatively, if tmout!=0, then SQLite has already waited 
+      ** sqlite3.busyTimeout ms for a lock. In this case, return 0 to 
+      ** indicate that the lock should not be retried and the SQLITE_BUSY 
+      ** error returned to the application.  */
+      if( tmout ){
+        tmout = 0;
+        sqlite3OsFileControl(pFile, SQLITE_FCNTL_LOCK_TIMEOUT, &tmout);
+        return 0;
+      }
     }else{
       return 1;
     }
diff --git a/src/os_unix.c b/src/os_unix.c
index cf1c579fd0..a4dd2f906c 100644
--- a/src/os_unix.c
+++ b/src/os_unix.c
@@ -3995,7 +3995,9 @@ static int unixFileControl(sqlite3_file *id, int op, void *pArg){
     }
 #ifdef SQLITE_ENABLE_SETLK_TIMEOUT
     case SQLITE_FCNTL_LOCK_TIMEOUT: {
+      int iOld = pFile->iBusyTimeout;
       pFile->iBusyTimeout = *(int*)pArg;
+      *(int*)pArg = iOld;
       return SQLITE_OK;
     }
 #endif
@@ -4817,6 +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.
+  **
+  **   1. Checkpointer lock (ofst==1).
+  **   2. Recover lock (ofst==2).
+  **   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
+  );
+#endif
+
   mask = (1<<(ofst+n)) - (1<<ofst);
   assert( n>1 || mask==(1<<ofst) );
   sqlite3_mutex_enter(pShmNode->pShmMutex);
diff --git a/src/sqlite.h.in b/src/sqlite.h.in
index 7b2049ae0a..86c3223801 100644
--- a/src/sqlite.h.in
+++ b/src/sqlite.h.in
@@ -1087,10 +1087,12 @@ struct sqlite3_io_methods {
 ** a prior successful call to [SQLITE_FCNTL_BEGIN_ATOMIC_WRITE].
 **
 ** <li>[[SQLITE_FCNTL_LOCK_TIMEOUT]]
-** The [SQLITE_FCNTL_LOCK_TIMEOUT] opcode causes attempts to obtain
-** a file lock using the xLock or xShmLock methods of the VFS to wait
-** for up to M milliseconds before failing, where M is the single 
-** unsigned integer parameter.
+** The [SQLITE_FCNTL_LOCK_TIMEOUT] opcode is used to configure a VFS
+** to block for up to M milliseconds before failing when attempting to 
+** obtain a file lock using the xLock or xShmLock methods of the VFS. 
+** The parameter is a pointer to a 32-bit signed integer that contains
+** the value that M is to be set to. Before returning, the 32-bit signed
+** integer is overwritten with the previous value of M.
 **
 ** <li>[[SQLITE_FCNTL_DATA_VERSION]]
 ** The [SQLITE_FCNTL_DATA_VERSION] opcode is used to detect changes to
diff --git a/src/wal.c b/src/wal.c
index 034a1624ea..e6742f1c44 100644
--- a/src/wal.c
+++ b/src/wal.c
@@ -1140,6 +1140,11 @@ static int walIndexRecover(Wal *pWal){
   u32 aFrameCksum[2] = {0, 0};
   int iLock;                      /* Lock offset to lock for checkpoint */
 
+#ifdef SQLITE_ENABLE_SETLK_TIMEOUT
+  int tmout = 0;
+  sqlite3OsFileControl(pWal->pDbFd, SQLITE_FCNTL_LOCK_TIMEOUT, (void*)&tmout);
+#endif
+
   /* Obtain an exclusive lock on all byte in the locking range not already
   ** locked by the caller. The caller is guaranteed to have locked the
   ** WAL_WRITE_LOCK byte, and may have also locked the WAL_CKPT_LOCK byte.
@@ -2758,6 +2763,9 @@ int sqlite3WalSnapshotRecover(Wal *pWal){
 int sqlite3WalBeginReadTransaction(Wal *pWal, int *pChanged){
   int rc;                         /* Return code */
   int cnt = 0;                    /* Number of TryBeginRead attempts */
+#ifdef SQLITE_ENABLE_SETLK_TIMEOUT
+  int tmout = 0;
+#endif
 
 #ifdef SQLITE_ENABLE_SNAPSHOT
   int bChanged = 0;
@@ -2767,6 +2775,12 @@ int sqlite3WalBeginReadTransaction(Wal *pWal, int *pChanged){
   }
 #endif
 
+#ifdef SQLITE_ENABLE_SETLK_TIMEOUT
+  /* Disable blocking locks. They are not useful when trying to open a
+  ** read-transaction, and blocking may cause deadlock anyway. */
+  sqlite3OsFileControl(pWal->pDbFd, SQLITE_FCNTL_LOCK_TIMEOUT, (void*)&tmout);
+#endif
+
   do{
     rc = walTryBeginRead(pWal, pChanged, 0, ++cnt);
   }while( rc==WAL_RETRY );
@@ -2775,6 +2789,16 @@ int sqlite3WalBeginReadTransaction(Wal *pWal, int *pChanged){
   testcase( rc==SQLITE_PROTOCOL );
   testcase( rc==SQLITE_OK );
 
+#ifdef SQLITE_ENABLE_SETLK_TIMEOUT
+  /* If they were disabled earlier and the read-transaction has been
+  ** successfully opened, re-enable blocking locks. This is because the
+  ** connection may attempt to upgrade to a write-transaction, which does
+  ** benefit from using blocking locks.  */
+  if( rc==SQLITE_OK ){
+    sqlite3OsFileControl(pWal->pDbFd, SQLITE_FCNTL_LOCK_TIMEOUT, (void*)&tmout);
+  }
+#endif
+
 #ifdef SQLITE_ENABLE_SNAPSHOT
   if( rc==SQLITE_OK ){
     if( pSnapshot && memcmp(pSnapshot, &pWal->hdr, sizeof(WalIndexHdr))!=0 ){