]> git.ipfire.org Git - thirdparty/sqlite.git/commitdiff
When ENABLE_SETLK is defined, avoid ever blocking on the lock mutex in os_unix.c...
authordan <Dan Kennedy>
Sat, 18 Nov 2023 17:20:04 +0000 (17:20 +0000)
committerdan <Dan Kennedy>
Sat, 18 Nov 2023 17:20:04 +0000 (17:20 +0000)
FossilOrigin-Name: eb36d475e91bfdbf4a18b6fd9751fbcecf15d960dcd1c00d2d18b5bf1d7503fe

manifest
manifest.uuid
src/os_unix.c

index b090e67c564547c8ab15f37b44b387856c2b3494..448e0070950ab8c56bb9775477e6fb07c425378e 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Adjust\san\sassert()\sin\sfts5WritePoslistData()\sso\sthat\sit\sonly\sapplies\sif\sthere\nhave\sbeen\sno\sprior\serrors.\ndbsqlfuzz\s25dca9b2568f67dc78a0e32ff280133fe71994bd.
-D 2023-11-18T12:06:21.202
+C When\sENABLE_SETLK\sis\sdefined,\savoid\sever\sblocking\son\sthe\slock\smutex\sin\sos_unix.c\swhen\srequesting\san\sexclusive\slock.
+D 2023-11-18T17:20:04.644
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -709,7 +709,7 @@ F src/os.h 1ff5ae51d339d0e30d8a9d814f4b8f8e448169304d83a7ed9db66a65732f3e63
 F src/os_common.h 6c0eb8dd40ef3e12fe585a13e709710267a258e2c8dd1c40b1948a1d14582e06
 F src/os_kv.c 4d39e1f1c180b11162c6dc4aa8ad34053873a639bac6baae23272fc03349986a
 F src/os_setup.h 6011ad7af5db4e05155f385eb3a9b4470688de6f65d6166b8956e58a3d872107
-F src/os_unix.c 1f6a930e8469b3709728690d089b55deb2fe605ba860941ddc21a2345e51bce4
+F src/os_unix.c dc5404b56da7fb13cf272ddb94c3753cf9e82d32a65cba35dbb6aadcb849419c
 F src/os_win.c 4a50a154aeebc66a1f8fb79c1ff6dd5fe3d005556533361e0d460d41cb6a45a8
 F src/os_win.h 7b073010f1451abe501be30d12f6bc599824944a
 F src/pager.c 987ab3a2cd9065d62e9955474470ff733445e2357432a67e3d0f5a8f9313e334
@@ -2140,8 +2140,8 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P 3afaeac56dff58db596360bf6f8dca97cb31405f73be8e189e8c0e6a1e5b239d
-R 9cf44b62577ea28a6c89efc96ebe6c29
-U drh
-Z 76fce07162fdeb3d4e5e596aa33e5ef1
+P 257cdbab90c6db8ccc9a8fd5df556b69c3a35a329d39cd4642c792d7359a54a5
+R c4cd41341f8782719a82292d54af9847
+U dan
+Z d62588e5cdbd2512931321f6949cd27c
 # Remove this line to create a well-formed Fossil manifest.
index f8f8ebe658089a85e7c451f5342568163ca1f8c9..92300531cbce63219f485d2e48a61791e257ccf6 100644 (file)
@@ -1 +1 @@
-257cdbab90c6db8ccc9a8fd5df556b69c3a35a329d39cd4642c792d7359a54a5
\ No newline at end of file
+eb36d475e91bfdbf4a18b6fd9751fbcecf15d960dcd1c00d2d18b5bf1d7503fe
\ No newline at end of file
index 82c81a4b1272084c89fe83bb174e3e104a6b83a9..dab03c97fb1afb3c912dede4d9cfa69e282db37f 100644 (file)
@@ -5056,95 +5056,115 @@ static int unixShmLock(
    || (flags==(SQLITE_SHM_EXCLUSIVE|SQLITE_SHM_LOCK))
   ){
 
-    /* Take the required mutexes */
+    /* Take the required mutexes. In SETLK_TIMEOUT mode (blocking locks), if
+    ** this is an attempt on an exclusive lock use sqlite3_mutex_try(). If any
+    ** other thread is holding this mutex, then it is either holding or about
+    ** to hold a lock exclusive to the one being requested, and we may
+    ** therefore return SQLITE_BUSY to the caller.
+    **
+    ** Doing this prevents some deadlock scenarios. For example, thread 1 may
+    ** be a checkpointer blocked waiting on the WRITER lock. And thread 2
+    ** may be a normal SQL client upgrading to a write transaction. In this
+    ** case thread 2 does a non-blocking request for the WRITER lock. But -
+    ** if it were to use sqlite3_mutex_enter() then it would effectively
+    ** become a (doomed) blocking request, as thread 2 would block until thread
+    ** 1 obtained WRITER and released the mutex. Since thread 2 already holds
+    ** a lock on a read-locking slot at this point, this breaks the
+    ** anti-deadlock rules (see above).  */
 #ifdef SQLITE_ENABLE_SETLK_TIMEOUT
     int iMutex;
     for(iMutex=ofst; iMutex<ofst+n; iMutex++){
-      sqlite3_mutex_enter(pShmNode->aMutex[iMutex]);
+      if( flags==(SQLITE_SHM_LOCK|SQLITE_SHM_EXCLUSIVE) ){
+        rc = sqlite3_mutex_try(pShmNode->aMutex[iMutex]);
+        if( rc!=SQLITE_OK ) break;
+      }else{
+        sqlite3_mutex_enter(pShmNode->aMutex[iMutex]);
+      }
     }
 #else
     sqlite3_mutex_enter(pShmNode->pShmMutex);
 #endif
 
-    if( flags & SQLITE_SHM_UNLOCK ){
-      /* Case (a) - unlock.  */
-      int bUnlock = 1;
-      assert( (p->exclMask & p->sharedMask)==0 );
-      assert( (flags & SQLITE_SHM_EXCLUSIVE)==0 || (p->exclMask & mask)==mask );
-      assert( (flags & SQLITE_SHM_SHARED)==0 || (p->sharedMask & mask)==mask );
-
-      /* If this is a SHARED lock being unlocked, it is possible that other
-      ** clients within this process are holding the same SHARED lock. In
-      ** this case, set bUnlock to 0 so that the posix lock is not removed
-      ** from the file-descriptor below.  */
-      if( flags & SQLITE_SHM_SHARED ){
-        assert( n==1 );
-        assert( aLock[ofst]>=1 );
-        if( aLock[ofst]>1 ){
-          bUnlock = 0;
-          aLock[ofst]--;
-          p->sharedMask &= ~mask;
+    if( rc==SQLITE_OK ){
+      if( flags & SQLITE_SHM_UNLOCK ){
+        /* Case (a) - unlock.  */
+        int bUnlock = 1;
+        assert( (p->exclMask & p->sharedMask)==0 );
+        assert( !(flags & SQLITE_SHM_EXCLUSIVE) || (p->exclMask & mask)==mask );
+        assert( !(flags & SQLITE_SHM_SHARED) || (p->sharedMask & mask)==mask );
+  
+        /* If this is a SHARED lock being unlocked, it is possible that other
+        ** clients within this process are holding the same SHARED lock. In
+        ** this case, set bUnlock to 0 so that the posix lock is not removed
+        ** from the file-descriptor below.  */
+        if( flags & SQLITE_SHM_SHARED ){
+          assert( n==1 );
+          assert( aLock[ofst]>=1 );
+          if( aLock[ofst]>1 ){
+            bUnlock = 0;
+            aLock[ofst]--;
+            p->sharedMask &= ~mask;
+          }
         }
-      }
-
-      if( bUnlock ){
-        rc = unixShmSystemLock(pDbFd, F_UNLCK, ofst+UNIX_SHM_BASE, n);
-        if( rc==SQLITE_OK ){
-          memset(&aLock[ofst], 0, sizeof(int)*n);
-          p->sharedMask &= ~mask;
-          p->exclMask &= ~mask;
+  
+        if( bUnlock ){
+          rc = unixShmSystemLock(pDbFd, F_UNLCK, ofst+UNIX_SHM_BASE, n);
+          if( rc==SQLITE_OK ){
+            memset(&aLock[ofst], 0, sizeof(int)*n);
+            p->sharedMask &= ~mask;
+            p->exclMask &= ~mask;
+          }
         }
-      }
-    }else if( flags & SQLITE_SHM_SHARED ){
-      /* Case (b) - a shared lock.  */
-
-      if( aLock[ofst]<0 ){
-        /* An exclusive lock is held by some other connection. BUSY. */ 
-        rc = SQLITE_BUSY;
-      }else if( aLock[ofst]==0 ){
-        rc = unixShmSystemLock(pDbFd, F_RDLCK, ofst+UNIX_SHM_BASE, n);
-      }
-
-      /* Get the local shared locks */
-      if( rc==SQLITE_OK ){
-        p->sharedMask |= mask;
-        aLock[ofst]++;
-      }
-    }else{
-      /* Case (c) - an exclusive lock.  */
-      int ii;
-
-      assert( flags==(SQLITE_SHM_LOCK|SQLITE_SHM_EXCLUSIVE) );
-      assert( (p->sharedMask & mask)==0 );
-      assert( (p->exclMask & mask)==0 );
-
-      /* Make sure no sibling connections hold locks that will block this
-      ** lock.  If any do, return SQLITE_BUSY right away.  */
-      for(ii=ofst; ii<ofst+n; ii++){
-        if( aLock[ii] ){
+      }else if( flags & SQLITE_SHM_SHARED ){
+        /* Case (b) - a shared lock.  */
+  
+        if( aLock[ofst]<0 ){
+          /* An exclusive lock is held by some other connection. BUSY. */ 
           rc = SQLITE_BUSY;
-          break;
+        }else if( aLock[ofst]==0 ){
+          rc = unixShmSystemLock(pDbFd, F_RDLCK, ofst+UNIX_SHM_BASE, n);
         }
-      }
-
-      /* Get the exclusive locks at the system level. Then if successful
-      ** also update the in-memory values. */
-      if( rc==SQLITE_OK ){
-        rc = unixShmSystemLock(pDbFd, F_WRLCK, ofst+UNIX_SHM_BASE, n);
+  
+        /* Get the local shared locks */
         if( rc==SQLITE_OK ){
-          p->exclMask |= mask;
-          for(ii=ofst; ii<ofst+n; ii++){
-            aLock[ii] = -1;
+          p->sharedMask |= mask;
+          aLock[ofst]++;
+        }
+      }else{
+        /* Case (c) - an exclusive lock.  */
+        int ii;
+  
+        assert( flags==(SQLITE_SHM_LOCK|SQLITE_SHM_EXCLUSIVE) );
+        assert( (p->sharedMask & mask)==0 );
+        assert( (p->exclMask & mask)==0 );
+  
+        /* Make sure no sibling connections hold locks that will block this
+        ** lock.  If any do, return SQLITE_BUSY right away.  */
+        for(ii=ofst; ii<ofst+n; ii++){
+          if( aLock[ii] ){
+            rc = SQLITE_BUSY;
+            break;
+          }
+        }
+  
+        /* Get the exclusive locks at the system level. Then if successful
+        ** also update the in-memory values. */
+        if( rc==SQLITE_OK ){
+          rc = unixShmSystemLock(pDbFd, F_WRLCK, ofst+UNIX_SHM_BASE, n);
+          if( rc==SQLITE_OK ){
+            p->exclMask |= mask;
+            for(ii=ofst; ii<ofst+n; ii++){
+              aLock[ii] = -1;
+            }
           }
         }
       }
+      assert( assertLockingArrayOk(pShmNode) );
     }
-  
-    assert( assertLockingArrayOk(pShmNode) );
 
     /* Drop the mutexes acquired above. */
 #ifdef SQLITE_ENABLE_SETLK_TIMEOUT
-    for(iMutex=ofst; iMutex<ofst+n; iMutex++){
+    for(iMutex--; iMutex>=ofst; iMutex--){
       sqlite3_mutex_leave(pShmNode->aMutex[iMutex]);
     }
 #else