From: dan <Dan Kennedy>
Date: Tue, 10 Dec 2024 19:00:07 +0000 (+0000)
Subject: Ensure windows may block on a SHARED (but not EXCLUSIVE) DMS lock.
X-Git-Tag: major-release~248^2~20
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d50eb9c9995109c10f832792bfb7d33c5273e0f6;p=thirdparty%2Fsqlite.git

Ensure windows may block on a SHARED (but not EXCLUSIVE) DMS lock.

FossilOrigin-Name: 55e4e7bb0074859e8c3e736e7eceb69af8408ce75135f5baa09e98aaf4e2a321
---

diff --git a/manifest b/manifest
index f262cc8852..4f8171101c 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Fix\swalsetlk.test\sto\swork\son\sunix.
-D 2024-12-10T18:12:59.802
+C Ensure\swindows\smay\sblock\son\sa\sSHARED\s(but\snot\sEXCLUSIVE)\sDMS\slock.
+D 2024-12-10T19:00:07.376
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md e108e1e69ae8e8a59e93c455654b8ac9356a11720d3345df2a4743e9590fb20d
@@ -762,7 +762,7 @@ F src/os_common.h 6c0eb8dd40ef3e12fe585a13e709710267a258e2c8dd1c40b1948a1d14582e
 F src/os_kv.c 4d39e1f1c180b11162c6dc4aa8ad34053873a639bac6baae23272fc03349986a
 F src/os_setup.h 6011ad7af5db4e05155f385eb3a9b4470688de6f65d6166b8956e58a3d872107
 F src/os_unix.c d2edbd92b07a3f778c2defa8a2e9d75acceb6267bda56948c41e8cdda65224d6
-F src/os_win.c 85c4d04e3d679a471c35172a58afa106040c26b9d6ba80be2efaf948c60be783
+F src/os_win.c c7a28dee9db7f4ca01aaf1f8561635870fc660581a5a7c8ecee6aa455829cf29
 F src/os_win.h 7b073010f1451abe501be30d12f6bc599824944a
 F src/pager.c 9656ad4e8331efb8a4f94f7a0c6440b98caea073950a367ea0c728a53b8e62c9
 F src/pager.h 4b1140d691860de0be1347474c51fee07d5420bd7f802d38cbab8ea4ab9f538a
@@ -2021,7 +2021,7 @@ F test/walro.test cb438d05ba0d191f10b688e39c4f0cd5b71569a1d1f4440e5bdf3c6880e08c
 F test/walro2.test 33955a6fd874dd9724005e17f77fef89d334b3171454a1256fe4941a96766cdc
 F test/walrofault.test c70cb6e308c443867701856cce92ad8288cd99488fa52afab77cca6cfd51af68
 F test/walseh1.test bae700eb99519b6d5cd3f893c04759accc5a59c391d4189fe4dd6995a533442b
-F test/walsetlk.test e24dd0a7bba843a3f7da97b6571d0e7fa9133941e74bb75a1d89e172008df31c
+F test/walsetlk.test 6e02311c7697cdc93610d24e8baaefe48592018cb5b317cba089c1bcd8d16f3a
 F test/walshared.test 42e3808582504878af237ea02c42ca793e8a0efaa19df7df26ac573370dbc7a3
 F test/walslow.test 0c51843836c9dcf40a5ac05aa781bfb977b396ee2c872d92bd48b79d5dd9aa23
 F test/walthread.test 14b20fcfa6ae152f5d8e12f5dc8a8a724b7ef189f5d8ef1e2ceab79f2af51747
@@ -2202,8 +2202,8 @@ F tool/version-info.c 3b36468a90faf1bbd59c65fd0eb66522d9f941eedd364fabccd7227350
 F tool/warnings-clang.sh bbf6a1e685e534c92ec2bfba5b1745f34fb6f0bc2a362850723a9ee87c1b31a7
 F tool/warnings.sh 49a486c5069de041aedcbde4de178293e0463ae9918ecad7539eedf0ec77a139
 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f
-P d67a42d62555cf9294e11a04d3c7cfdf11ca7978d60fc085f5a73e36aff8313e
-R f7af9d80a506351b36f5bd843c579062
+P 798dff041be46d2d5014779c0855f60e6b2a142e9946bfb79061d7dff0bc3d27
+R d7fe08eda9a97ac9dd36271e2e66b9ad
 U dan
-Z 1f09d6481c3b825c642e11d0b6e347e6
+Z 8b09a868e54db464087d18bec9270611
 # Remove this line to create a well-formed Fossil manifest.
diff --git a/manifest.uuid b/manifest.uuid
index 9e8542b6bf..3f3f3749c3 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-798dff041be46d2d5014779c0855f60e6b2a142e9946bfb79061d7dff0bc3d27
+55e4e7bb0074859e8c3e736e7eceb69af8408ce75135f5baa09e98aaf4e2a321
diff --git a/src/os_win.c b/src/os_win.c
index c5342b2d03..00866dbf67 100644
--- a/src/os_win.c
+++ b/src/os_win.c
@@ -292,6 +292,12 @@ struct winFile {
 #endif
 };
 
+#ifdef SQLITE_ENABLE_SETLK_TIMEOUT
+# define winFileBusyTimeout(pDbFd) pDbFd->iBusyTimeout
+#else
+# define winFileBusyTimeout(pDbFd) 0
+#endif
+
 /*
 ** The winVfsAppData structure is used for the pAppData member for all of the
 ** Win32 VFS variants.
@@ -2555,7 +2561,7 @@ static BOOL winLockFile(
 ** some other process holds the lock, SQLITE_BUSY is returned if nMs==0, or
 ** SQLITE_BUSY_TIMEOUT otherwise. Or, if an error occurs, SQLITE_IOERR.
 */
-static int winLockFileTimeout(
+static int winHandleLockTimeout(
   HANDLE hFile,
   DWORD offset,
   DWORD nByte,
@@ -2647,6 +2653,9 @@ static BOOL winUnlockFile(
 #endif
 }
 
+/*
+** Remove an nByte lock starting at offset iOff from HANDLE h.
+*/
 static int winHandleUnlock(HANDLE h, int iOff, int nByte){
   BOOL ret = winUnlockFile(&h, iOff, 0, nByte, 0);
   return (ret ? SQLITE_OK : SQLITE_IOERR_UNLOCK);
@@ -3867,12 +3876,17 @@ static int winShmMutexHeld(void) {
 **
 ** The following fields are read-only after the object is created:
 **
-**      hFile
 **      zFilename
 **
 ** Either winShmNode.mutex must be held or winShmNode.nRef==0 and
 ** winShmMutexHeld() is true when reading or writing any other field
 ** in this structure.
+**
+** File-handle hSharedShm is used to (a) take the DMS lock, (b) truncate
+** the *-shm file if the DMS-locking protocol demands it, and (c) map
+** regions of the *-shm file into memory using MapViewOfFile() or 
+** similar. Other locks are taken by individual clients using the
+** winShm.hShm handles.
 */
 struct winShmNode {
   sqlite3_mutex *mutex;      /* Mutex to access this object */
@@ -3907,16 +3921,8 @@ static winShmNode *winShmNodeList = 0;
 
 /*
 ** Structure used internally by this VFS to record the state of an
-** open shared memory connection.
-**
-** The following fields are initialized when this object is created and
-** are read-only thereafter:
-**
-**    winShm.pShmNode
-**    winShm.id
-**
-** All other fields are read/write.  The winShm.pShmNode->mutex must be held
-** while accessing any read/write fields.
+** open shared memory connection. There is one such structure for each
+** winFile open on a wal mode database.
 */
 struct winShm {
   winShmNode *pShmNode;      /* The underlying winShmNode object */
@@ -3991,11 +3997,12 @@ static void winShmPurge(sqlite3_vfs *pVfs, int deleteFlag){
 ** pShmNode. Take the lock. Truncate the *-shm file if required.
 ** Return SQLITE_OK if successful, or an SQLite error code otherwise.
 */
-static int winLockSharedMemory(winShmNode *pShmNode){
+static int winLockSharedMemory(winShmNode *pShmNode, int nMs){
   HANDLE h = pShmNode->hSharedShm;
   int rc = SQLITE_OK;
 
-  rc = winLockFileTimeout(h, WIN_SHM_DMS, 1, 1, 0);
+  assert( sqlite3_mutex_held(pShmNode->mutex) );
+  rc = winHandleLockTimeout(h, WIN_SHM_DMS, 1, 1, 0);
   if( rc==SQLITE_OK ){
     /* We have an EXCLUSIVE lock on the DMS byte. This means that this
     ** is the first process to open the file. Truncate it to zero bytes
@@ -4014,7 +4021,7 @@ static int winLockSharedMemory(winShmNode *pShmNode){
 
   if( rc==SQLITE_OK ){
     /* Take a SHARED lock on the DMS byte. */
-    rc = winLockFileTimeout(h, WIN_SHM_DMS, 1, 0, 0);
+    rc = winHandleLockTimeout(h, WIN_SHM_DMS, 1, 0, nMs);
     if( rc==SQLITE_OK ){
       pShmNode->isUnlocked = 0;
     }
@@ -4353,11 +4360,8 @@ static int winShmLock(
       }
     }else{
       int bExcl = ((flags & SQLITE_SHM_EXCLUSIVE) ? 1 : 0);
-      int nMs = 0;
-#ifdef SQLITE_ENABLE_SETLK_TIMEOUT
-      nMs = pDbFd->iBusyTimeout;
-#endif
-      rc = winLockFileTimeout(p->hShm, ofst+WIN_SHM_BASE, n, bExcl, nMs);
+      int nMs = winFileBusyTimeout(pDbFd);
+      rc = winHandleLockTimeout(p->hShm, ofst+WIN_SHM_BASE, n, bExcl, nMs);
       if( rc==SQLITE_OK ){
         if( bExcl ){
           p->exclMask = (p->exclMask | mask);
@@ -4435,7 +4439,7 @@ static int winShmMap(
   if( pShmNode->isUnlocked ){
     /* Take the DMS lock. */
     assert( pShmNode->nRegion==0 );
-    rc = winLockSharedMemory(pShmNode);
+    rc = winLockSharedMemory(pShmNode, winFileBusyTimeout(pDbFd));
     if( rc!=SQLITE_OK ) goto shmpage_out;
   }
 
diff --git a/test/walsetlk.test b/test/walsetlk.test
index 3a26d9a09a..d67d53dd42 100644
--- a/test/walsetlk.test
+++ b/test/walsetlk.test
@@ -18,7 +18,6 @@ source $testdir/lock_common.tcl
 set testprefix walsetlk
 
 ifcapable !wal {finish_test ; return }
-if 0 {
 db timeout 1000
 
 #-------------------------------------------------------------------------
@@ -90,7 +89,6 @@ do_multiclient_test tn {
 
   set ::sleep_count 0
   proc xSleep_callback {xSleep nMs} {
-  puts POOOOOOP
     after [expr $nMs / 1000]
     incr ::sleep_count
   }
@@ -220,7 +218,6 @@ do_multiclient_test tn {
 }
 
 #-------------------------------------------------------------------------
-}
 reset_db
 
 testvfs tvfs -fullshm 1