From 7f13746e4cdfff4a06e962f92fc5a5770f855dad Mon Sep 17 00:00:00 2001 From: stephan Date: Wed, 30 Sep 2026 11:56:05 +0000 Subject: [PATCH] Fix the OPFS SAHPool VFS's xLock/xUnlock to properly deal with multiple handles to the same underlying file within the same JS thread. Addresses [forum:5e66a600ba6e57de|bug report 2026-09-30T10:49:15Z]. Also (unrelated but code-adjacent) make sure that the VFS releases all of its internal mappings when it's paused. FossilOrigin-Name: 079400229e098b807143001c0e534b48e8dcec6c9171d6c10d8bec0c59254ec4 --- ext/wasm/api/sqlite3-vfs-opfs-sahpool.c-pp.js | 88 +++++++++++++++---- manifest | 14 +-- manifest.uuid | 2 +- 3 files changed, 81 insertions(+), 23 deletions(-) diff --git a/ext/wasm/api/sqlite3-vfs-opfs-sahpool.c-pp.js b/ext/wasm/api/sqlite3-vfs-opfs-sahpool.c-pp.js index de1d6d6fd1..ada1215c9b 100644 --- a/ext/wasm/api/sqlite3-vfs-opfs-sahpool.c-pp.js +++ b/ext/wasm/api/sqlite3-vfs-opfs-sahpool.c-pp.js @@ -205,6 +205,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ if(file) { try{ pool.log(`xClose ${file.path}`); + pool.unlockFile(file, capi.SQLITE_LOCK_NONE); pool.mapS3FileToOFile(pFile, false); file.sah.flush(); if(file.flags & capi.SQLITE_OPEN_DELETEONCLOSE){ @@ -236,8 +237,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ pool.log(`xLock ${lockType}`); pool.storeErr(); const file = pool.getOFileForS3File(pFile); - file.lockType = lockType; - return 0; + return pool.lockFile(file, lockType); }, xRead: function(pFile,pDest,n,offset64){ const pool = getPoolForPFile(pFile); @@ -291,7 +291,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ const pool = getPoolForPFile(pFile); pool.log('xUnlock'); const file = pool.getOFileForS3File(pFile); - file.lockType = lockType; + pool.unlockFile(file, lockType); return 0; }, xWrite: function(pFile,pSrc,n,offset64){ @@ -506,7 +506,11 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ /* Set of currently-unused SAHs. */ #availableSAH = new Set(); /* Maps (sqlite3_file*) to xOpen's file objects. */ - #mapS3FileToOFile_ = new Map(); + #mapS3FileToOFile = new Map(); + /* Maps client-side file names to the lock state of the files + opened via this pool: {nShared, lockType}, modelled after + os_unix.c:unixInodeInfo. */ + #mapPathToLock = new Map(); /* Maps SAH to an abstract File Object which contains various metadata about that handle. */ @@ -613,6 +617,8 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ this.#mapSAHToName.clear(); this.#mapFilenameToSAH.clear(); this.#availableSAH.clear(); + this.#mapS3FileToOFile.clear(); + this.#mapPathToLock.clear(); } /** @@ -864,7 +870,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ xOpen file object. */ getOFileForS3File(pFile){ - return this.#mapS3FileToOFile_.get(pFile); + return this.#mapS3FileToOFile.get(pFile); } /** Maps or unmaps (if file is falsy) the given (sqlite3_file*) @@ -872,10 +878,10 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ */ mapS3FileToOFile(pFile,file){ if(file){ - this.#mapS3FileToOFile_.set(pFile, file); + this.#mapS3FileToOFile.set(pFile, file); setPoolForPFile(pFile, this); }else{ - this.#mapS3FileToOFile_.delete(pFile); + this.#mapS3FileToOFile.delete(pFile); setPoolForPFile(pFile, false); } } @@ -955,7 +961,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ @see unpauseVfs() */ pauseVfs(){ - if(this.#mapS3FileToOFile_.size>0){ + if(this.#mapS3FileToOFile.size>0){ sqlite3.SQLite3Error.toss( capi.SQLITE_MISUSE, "Cannot pause VFS", this.vfsName,"because it has opened files." @@ -1083,14 +1089,66 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ its opaque name) holds a RESERVED or greater lock, else false. */ hasReservedLock(filename){ - const lr = capi.SQLITE_LOCK_RESERVED; - for(const f of this.#mapS3FileToOFile_.values()){ - if(f.lockType>=lr && filename===f.path){ - return true; - } - } - return false; + const lk = this.#mapPathToLock.get(filename); + return !!lk && lk.lockType>capi.SQLITE_LOCK_SHARED; } + + + /** + Implements xLock() for the given xOpen-generated file object. + This pool holds its SAHs exclusively, so its xOpen() represent + the only connections to a given file. The locks are held + separately from their file object in the same thread the same + way in which os_unix.c:unixLock() distinguishes multiple + connections of a single process which share an inode: a lock + which conflicts with another connection's lock fails with + SQLITE_BUSY, and a failed attempt to get from RESERVED to + EXCLUSIVE leaves a PENDING lock, which admits no new SHARED + locks. + */ + lockFile(file, lockType){ + if(file.lockType>=lockType) return 0; + let lk = this.#mapPathToLock.get(file.path); + if(!lk){ + lk = Object.assign(Object.create(null),{ + nShared: 0, lockType: capi.SQLITE_LOCK_NONE + }); + this.#mapPathToLock.set(file.path, lk); + } + if(file.lockType!==lk.lockType + && (lk.lockType>=capi.SQLITE_LOCK_PENDING + || lockType>capi.SQLITE_LOCK_SHARED)){ + return capi.SQLITE_BUSY; + } + if(lockType===capi.SQLITE_LOCK_SHARED){ + ++lk.nShared; + if(lk.lockType===capi.SQLITE_LOCK_NONE) lk.lockType = lockType; + }else if(lockType===capi.SQLITE_LOCK_EXCLUSIVE && lk.nShared>1){ + if(file.lockType===capi.SQLITE_LOCK_RESERVED){ + file.lockType = lk.lockType = capi.SQLITE_LOCK_PENDING; + } + return capi.SQLITE_BUSY; + }else{ + lk.lockType = lockType; + } + file.lockType = lockType; + return 0; + } + + /** Implements xUnlock() for the given xOpen file object. */ + unlockFile(file, lockType){ + if(file.lockType<=lockType) return; + const lk = this.#mapPathToLock.get(file.path); + if(file.lockType>capi.SQLITE_LOCK_SHARED){ + lk.lockType = capi.SQLITE_LOCK_SHARED; + } + if(lockType===capi.SQLITE_LOCK_NONE && 0===--lk.nShared + /* Maintenance reminder: the && ordering is significant */){ + this.#mapPathToLock.delete(file.path); + } + file.lockType = lockType; + } + }/*class OpfsSAHPool*/; /** diff --git a/manifest b/manifest index 77ba93f04c..e7f9173dfc 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Correct\sthe\sxCheckReserveLock()\simpl\sof\sthe\sOPFS\sSAHPool\sVFS\sto\sfix\sa\scorruption\scase\sinvolving\smultiple\shandles\sin\sthe\ssame\sJS\sthread\sto\sthe\ssame\sfilename\swhen\sone\sleaves\sa\sjournal\sbehind.\sAddresses\s[forum:b2fbb61642|bug\sreport\s2026-09-29T18:25:28Z]. -D 2026-09-30T10:37:55.170 +C Fix\sthe\sOPFS\sSAHPool\sVFS's\sxLock/xUnlock\sto\sproperly\sdeal\swith\smultiple\shandles\sto\sthe\ssame\sunderlying\sfile\swithin\sthe\ssame\sJS\sthread.\sAddresses\s[forum:5e66a600ba6e57de|bug\sreport\s2026-09-30T10:49:15Z].\sAlso\s(unrelated\sbut\scode-adjacent)\smake\ssure\sthat\sthe\sVFS\sreleases\sall\sof\sits\sinternal\smappings\swhen\sit's\spaused. +D 2026-09-30T11:56:05.637 F .fossil-settings/binary-glob 61195414528fb3ea9693577e1980230d78a1f8b0a54c78cf1b9b24d0a409ed6a x F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea @@ -606,7 +606,7 @@ F ext/wasm/api/sqlite3-license-version-header.js 98d90255a12d02214db634e041c8e7f F ext/wasm/api/sqlite3-opfs-async-proxy.c-pp.js 25e31482b04293a33d7599f1459eb552b3eb36ca10c02c816122d3308bf80cb2 F ext/wasm/api/sqlite3-vfs-helper.c-pp.js 3f828cc66758acb40e9c5b4dcfd87fd478a14c8fb7f0630264e6c7fa0e57515d F ext/wasm/api/sqlite3-vfs-kvvfs.c-pp.js e3758c4ef4375e4b65e847c7444532a3a15389855599cd33233fb4aff4bdfcd4 -F ext/wasm/api/sqlite3-vfs-opfs-sahpool.c-pp.js 0c3b30bd161f07201c36c06ce5d0c1b6de74e2ee85d334dc93beb86ffcd8a7ab +F ext/wasm/api/sqlite3-vfs-opfs-sahpool.c-pp.js c90f0ec0543a6a5c6a7aa0db089d6a324910490292c829f3c7160ef2c5fec19d F ext/wasm/api/sqlite3-vfs-opfs-wl.c-pp.js 3dbd918ef037cd8fa9c7b4dccb3c8637b228638654c429e7df6acab5981c75e2 F ext/wasm/api/sqlite3-vfs-opfs.c-pp.js 3da8fe72dc9e76614a9c102b92e777ce03f81d788b607701c828d8fcbac44b06 F ext/wasm/api/sqlite3-vtab-helper.c-pp.js 366596d8ff73d4cefb938bbe95bc839d503c3fab6c8335ce4bf52f0d8a7dee81 @@ -2207,9 +2207,9 @@ F tool/warnings-clang.sh bbf6a1e685e534c92ec2bfba5b1745f34fb6f0bc2a362850723a9ee F tool/warnings.sh a554d13f6e5cf3760f041b87939e3d616ec6961859c3245e8ef701d1eafc2ca2 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f F tool/winmain.c 00c8fb88e365c9017db14c73d3c78af62194d9644feaf60e220ab0f411f3604c -P 2a2ed4dc5a9085f213e3d3aeb0532b3898dae2e70f3f165c5c9b5c695e403dfb -Q +9168a6f1bedba87117169d096f9a6fcadaac65249d707193e4bcbc25e59ccb54 -R 8b05f80be3ee153230dd7ef996a83c19 +P ea1d55e202e6e6212d3d2286895ca343b820b96b50e6281ebaf1d23514684811 +Q +9e2caaa382cee5d5df26ad3af7d727ab3f68a912e39655ffcc52f592d5a08a8d +R 95f676ce87296e1c8d340b8373c77ee7 U stephan -Z 96a14566fcf21a9104b3009ca9d027ea +Z 23dc01e62652e0aa6b18abf5484a38f7 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 86c9ed7fda..750a26081c 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -ea1d55e202e6e6212d3d2286895ca343b820b96b50e6281ebaf1d23514684811 +079400229e098b807143001c0e534b48e8dcec6c9171d6c10d8bec0c59254ec4