Rework the fts5 db-close cleanup to avoid that fts5 internally calls a just-unbound...

FossilOrigin-Name: 795f22421b96cea27b3883a580d8ce29cc50dfb7d4f852b3d36ede3766c7a24c

diff --git a/ext/wasm/api/sqlite3-api-glue.js b/ext/wasm/api/sqlite3-api-glue.js
index 7f05d374430343ed97faf161e9124914091d10f5..b68baabae90b55bca59a7a6ea1c47e852249bf59 100644 (file)
--- a/ext/wasm/api/sqlite3-api-glue.js
+++ b/ext/wasm/api/sqlite3-api-glue.js
@@ -1069,15 +1069,32 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
      the being-destroyed (sqlite3*) object.
   */
   __dbCleanupMap.extraCallbacks = [];
+  /**
+     Downstream code, namely sqlite3-fts5-helper.js, should add any
+     custom cleanup handlers to __dbCleanupMap.postCloseCallbacks.
+     Each function in this array will be called during
+     sqlite3_close_v2(), AFTER the db is closed, and passed a pointer
+     to the being-destroyed (sqlite3*) object. The memory is NOT A
+     VALID OBJECT but its address is still valid as a lookup key.
+  */
+  __dbCleanupMap.postCloseCallbacks = [];
 
   {/* Binding of sqlite3_close_v2() */
     const __sqlite3CloseV2 = wasm.xWrap("sqlite3_close_v2", "int", "sqlite3*");
+    const __xArgDb = wasm.xWrap.argAdapter('sqlite3*');
     capi.sqlite3_close_v2 = function(pDb){
       if(1!==arguments.length) return __dbArgcMismatch(pDb, 'sqlite3_close_v2', 1);
+      pDb = __xArgDb(pDb);
       if(pDb){
         try{__dbCleanupMap.cleanup(pDb)} catch(e){/*ignored*/}
       }
-      return __sqlite3CloseV2(pDb);
+      const rc = __sqlite3CloseV2(pDb);
+      if(pDb/*noting that it's not valid anymore*/){
+        for(const f of __dbCleanupMap.postCloseCallbacks){
+          try{f(pDb)}catch(e){/*ingored*/}
+        }
+      }
+      return rc;
     };
   }/*sqlite3_close_v2()*/
 

diff --git a/ext/wasm/api/sqlite3-fts5-helper.js b/ext/wasm/api/sqlite3-fts5-helper.js
index 061b7ca20ff90e8b6c97775353da5ef66a183de4..09e3db8bed24d54a53ddfb1660eb5e74be274970 100644 (file)
--- a/ext/wasm/api/sqlite3-fts5-helper.js
+++ b/ext/wasm/api/sqlite3-fts5-helper.js
@@ -16,14 +16,15 @@
 'use strict';
 globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
   const wasm = sqlite3.wasm, capi = sqlite3.capi, toss = sqlite3.util.toss3;
-  const __dbCleanupMap = sqlite3.__dbCleanupMap
-        || toss("Maintenance needed: can't reach sqlite3.__dbCleanupMap.");
   if(!capi.fts5_api_from_db){
     return /*this build does not have FTS5*/;
   }
   const fts = sqlite3.fts5 = Object.create(null);
+  const xArgDb = wasm.xWrap.argAdapter('sqlite3*');
+
   /**
-     Move FTS-specific APIs from sqlite3.capi to sqlite3.fts.
+     Move FTS-specific APIs (installed via automation) from
+     sqlite3.capi to sqlite3.fts.
   */
   for(const c of [
     'Fts5ExtensionApi', 'Fts5PhraseIter', 'fts5_api',
@@ -33,8 +34,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
     delete capi[c];
   }
 
-  const cache = Object.create(null);
-
+  /* JS-to-WASM arg adapter for xCreateFunction()'s xFunction arg. */
   const xFunctionArgAdapter = new wasm.xWrap.FuncPtrAdapter({
     name: 'fts5_api::xCreateFunction(xFunction)',
     signature: 'v(pppip)',
@@ -56,6 +56,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
     }
   });
 
+  /* JS-to-WASM arg adapter for xCreateFunction()'s xDestroy arg. */
   const xDestroyArgAdapter = new wasm.xWrap.FuncPtrAdapter({
     name: 'fts5_api::xCreateFunction(xDestroy)',
     signature: 'v(p)',
@@ -70,59 +71,62 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
     }
   });
 
+
+  /** Map of (sqlite3*) to fts.fts5_api. */
+  const __ftsApiToStruct = Object.create(null);
+  const __fts5_api_from_db = function(pDb, createIfNeeded){
+    let rc = __ftsApiToStruct[pDb];
+    if(!rc && createIfNeeded){
+      rc = new fts.fts5_api(fts.fts5_api_from_db(pDb));
+      __ftsApiToStruct[pDb] = rc;
+    }
+    return rc;
+  };
+
   /**
      Arrange for WASM functions dynamically created via this API to be
      uninstalled when the db they were installed for is closed... */
-  const __fts5FuncsPerDb = Object.create(null);
-  const __cleanupForDb = function(pDb){
-    return (__fts5FuncsPerDb[pDb] || (__fts5FuncsPerDb[pDb] = new Set));
-  };
-  __dbCleanupMap._addFts5Function = function ff(pDb, name){
-    __cleanupForDb(pDb).add(name.toLowerCase());
+  const __addCleanupForFunc = function(sfapi, name){
+    if(!sfapi.$$funcNames){
+      sfapi.$$funcNames = new Set;
+    }
+    sfapi.$$funcNames.add(name.toLowerCase());
   };
-  __dbCleanupMap.extraCallbacks.push(function(pDb){
-    const list = __fts5FuncsPerDb[pDb];
-    if(list){
-      const fapi = fts.fts5_api_from_db(pDb)
-      const sfapi = new fts.fts5_api(fapi);
-      const scope = wasm.scopedAllocPush();
-      //wasm.xWrap.FuncPtrAdapter.debugFuncInstall = true;
-      try{
-        const xCF = wasm.functionEntry(sfapi.$xCreateFunction);
-        for(const name of list){
-          //sqlite3.config.warn("Clearing FTS function",pDb,name);
-          try{
-            const zName = wasm.scopedAllocCString(name);
-            /* In order to clean up properly we first have to invoke
-               the low-level (unwrapped) sfapi.$xCreateFunction and
-               then fake some calls into the Function-to-funcptr
-               apapters. If we just call cache.xcf(...) then the
-               adapter ends up uninstalling the xDestroy pointer which
-               sfapi.$xCreateFunction is about to invoke, leading to
-               an invalid memory access. */
-            xCF(fapi, zName, 0, 0, 0);
-            // Now clean up our FuncPtrAdapters in a round-about way...
-            const argv = [fapi, zName, 0, 0, 0];
-            xFunctionArgAdapter.convertArg(argv[3], argv, 3);
-            // xDestroyArgAdapter leads to an invalid memory access
-            // for as-yet-unknown reasons. For the time being we're
-            // leaking these.
-            //xDestroyArgAdapter.convertArg(argv[4], argv, 4);
-          }catch(e){
-            sqlite3.config.error("Error removing FTS func",name,e);
+
+  /**
+     Callback to be invoked via the JS binding of sqlite3_close_v2(),
+     after the db has been closed, meaing that the argument to this
+     function is not a valid object. We use its address only as a
+     lookup key.
+  */
+  sqlite3.__dbCleanupMap.postCloseCallbacks.push(function(pDb){
+    const sfapi = __fts5_api_from_db(pDb, false);
+    if(sfapi){
+      delete __ftsApiToStruct[pDb];
+      if(sfapi.$$funcNames){
+        const fapi = sfapi.pointer;
+        const scope = wasm.scopedAllocPush();
+        //wasm.xWrap.FuncPtrAdapter.debugFuncInstall = true;
+        try{
+          for(const name of sfapi.$$funcNames){
+            try{
+              const zName = wasm.scopedAllocCString(name);
+              const argv = [fapi, zName, 0, 0, 0];
+              xFunctionArgAdapter.convertArg(0, argv, 3);
+              xDestroyArgAdapter.convertArg(0, argv, 4);
+            }catch(e){
+              sqlite3.config.error("Error removing FTS func",name,e);
+            }
           }
+        }finally{
+          wasm.scopedAllocPop(scope);
         }
-      }finally{
-        sfapi.dispose();
-        delete __fts5FuncsPerDb[pDb];
-        wasm.scopedAllocPop(scope);
+        //wasm.xWrap.FuncPtrAdapter.debugFuncInstall = false;
       }
-      //wasm.xWrap.FuncPtrAdapter.debugFuncInstall = false;
+      sfapi.dispose();
     }
   });
 
-  const xArgDb = wasm.xWrap.argAdapter('sqlite3*');
-
   /**
      Convenience wrapper to fts5_api::xCreateFunction.
 
@@ -166,19 +170,19 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
     if( !name || 'string' !== typeof name ) toss("Invalid name argument.");
     const fapi = fts.fts5_api_from_db(db)
           || toss("Internal error - cannot get FTS5 API object for db.");
-    const sfapi = new fts.fts5_api(fapi);
+    const sfapi = __fts5_api_from_db(db, true);
     try{
-      const xcf = cache.xcf || (
-        cache.xcf = wasm.xWrap(sfapi.$xCreateFunction,'int',[
+      const xcf = sfapi.$$xCreateFunction || (
+        sfapi.$$xCreateFunction = wasm.xWrap(sfapi.$xCreateFunction, 'int', [
           '*', 'string', '*', xFunctionArgAdapter, xDestroyArgAdapter
         ])
-      )/* this cache entry is only legal as long as nobody replaces
-          the xCreateFunction() method */;
+      );
       const rc = xcf(fapi, name, 0, xFunction || 0, xDestroy || 0 );
       if(rc) toss(rc,"FTS5::xCreateFunction() failed.");
-      __dbCleanupMap._addFts5Function(db, name);
-    }finally{
+      __addCleanupForFunc(sfapi, name);
+    }catch(e){
       sfapi.dispose();
+      throw e;
     }
   };
 

diff --git a/ext/wasm/tester1.c-pp.js b/ext/wasm/tester1.c-pp.js
index 1e83d7e7c0735d864564b027ef5d2455c3fe9288..65e2be4a571242daeff7b668601065789dd64739 100644 (file)
--- a/ext/wasm/tester1.c-pp.js
+++ b/ext/wasm/tester1.c-pp.js
@@ -2921,7 +2921,7 @@ globalThis.sqlite3InitModule = sqlite3InitModule;
         fApi.dispose();
         fApi = undefined;
         let destroyCalled = false;
-        fts.createFunction(db,{
+        fts.createFunction(db, {
           name: 'mymatch',
           xFunction: function(pFtsX, pFtsCx, pCtx, argv){
             //log("mymatch!", argv.length, argv);
@@ -2939,8 +2939,6 @@ globalThis.sqlite3InitModule = sqlite3InitModule;
         });
         let list = db.selectValues(
           "select mymatch(ft,a,b) from ft where b match 'b2'"
-          //^^^ results in a "no such cursor: 0" error
-          //"select a from ft where b match 'b2'"
         );
         T.assert( 1 === list.length )
           .assert( 'MY+a2:b2' === list[0] );

diff --git a/manifest b/manifest
index 7db3bf87c73043982a694bd910082fb70a38d0c5..8c61ea30a1827ee4929bce71f236be53c7703017 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Part\s2\sof\sthe\sfix\sfrom\s[a0f808363318c00fd1db78b].
-D 2023-08-03T20:02:49.776
+C Rework\sthe\sfts5\sdb-close\scleanup\sto\savoid\sthat\sfts5\sinternally\scalls\sa\sjust-unbound-from-wasm\sxDestroy()\smethod.\sThere\sis\sstill\swork\sto\sdo\shere\sto\scover\sthe\scase\sof\sfts5\sfunctions\sbeing\sreplaced\s(in\swhich\sall\sxDestroy\smethods\shave\sto\sbe\sretained\suntil\sthe\sdb\sis\sclosed).
+D 2023-08-03T20:45:28.696
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -501,11 +501,11 @@ F ext/wasm/api/post-js-footer.js cd0a8ec768501d9bd45d325ab0442037fb0e33d1f3b4f08
 F ext/wasm/api/post-js-header.js 47b6b281f39ad59fa6e8b658308cd98ea292c286a68407b35ff3ed9cfd281a62
 F ext/wasm/api/pre-js.c-pp.js ad906703f7429590f2fbf5e6498513bf727a1a4f0ebfa057afb08161d7511219
 F ext/wasm/api/sqlite3-api-cleanup.js 8489ac9933783f36807cfa5d79bfa366008c0443b42bf47bdef41bbfce4aa367
-F ext/wasm/api/sqlite3-api-glue.js c8fee0a75cf655bb830f01b6d0a53e3648a46be71d69256b2853c085004bf89e
+F ext/wasm/api/sqlite3-api-glue.js 861ff5ccfbaa50579c46618be53ac374202ad73cc27eb5077967c35edbb1869d
 F ext/wasm/api/sqlite3-api-oo1.js 9678dc4d9a5d39632b6ffe6ea94a023119260815bf32f265bf5f6c36c9516db8
 F ext/wasm/api/sqlite3-api-prologue.js 76258e160bf6a89cc75a7d3c05646a054c8cab7219cd1e10bc20cacaad022131
 F ext/wasm/api/sqlite3-api-worker1.js 9f32af64df1a031071912eea7a201557fe39b1738645c0134562bb84e88e2fec
-F ext/wasm/api/sqlite3-fts5-helper.js fd9a0cad447a56734b134eab57a832be8820e8cfb8a672c0b1b54c3805485c2e
+F ext/wasm/api/sqlite3-fts5-helper.js 8d7a35e18463102fe2783657d5c59c3c0c7460f0eb699757ebfed2e44df2a001
 F ext/wasm/api/sqlite3-license-version-header.js 0c807a421f0187e778dc1078f10d2994b915123c1223fe752b60afdcd1263f89
 F ext/wasm/api/sqlite3-opfs-async-proxy.js 8cf8a897726f14071fae6be6648125162b256dfb4f96555b865dbb7a6b65e379
 F ext/wasm/api/sqlite3-v-helper.js 8dc3da6e69d51f455b64cc88fec7977f528d79ec267cfcdd97b606c8cc4cd156
@@ -556,7 +556,7 @@ F ext/wasm/test-opfs-vfs.html 1f2d672f3f3fce810dfd48a8d56914aba22e45c6834e262555
 F ext/wasm/test-opfs-vfs.js f09266873e1a34d9bdb6d3981ec8c9e382f31f215c9fd2f9016d2394b8ae9b7b
 F ext/wasm/tester1-worker.html ebc4b820a128963afce328ecf63ab200bd923309eb939f4110510ab449e9814c
 F ext/wasm/tester1.c-pp.html 1c1bc78b858af2019e663b1a31e76657b73dc24bede28ca92fbe917c3a972af2
-F ext/wasm/tester1.c-pp.js b1200f52a1b2ab51c0a2bc6be811090ee0a71e0d9661fcfe540073b80fb4d607
+F ext/wasm/tester1.c-pp.js 892d00acf7bf675e34766794dcc94bc26e5bf00abcfeeb9dbcc42eebac787bba
 F ext/wasm/tests/opfs/concurrency/index.html 0802373d57034d51835ff6041cda438c7a982deea6079efd98098d3e42fbcbc1
 F ext/wasm/tests/opfs/concurrency/test.js a98016113eaf71e81ddbf71655aa29b0fed9a8b79a3cdd3620d1658eb1cc9a5d
 F ext/wasm/tests/opfs/concurrency/worker.js 0a8c1a3e6ebb38aabbee24f122693f1fb29d599948915c76906681bb7da1d3d2
@@ -2051,8 +2051,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 a0f808363318c00fd1db78b4271cef8d05a046a36aab1a383e731e40603c6e2a
-R f0e23b22b28a37c03d5b2048de2190dd
+P ce0674b1925138f8f878b11aae0f8420bd968df0959f6dd7e208fb84bcbad07e
+R 55a64755ba6bd7e4b55de6971891bfd0
 U stephan
-Z 9f51c4a5e0c22d8a19d776fe443c68e1
+Z 971bc1dc41287f5b57d1768c8e038762
 # Remove this line to create a well-formed Fossil manifest.

diff --git a/manifest.uuid b/manifest.uuid
index 5044e3c62c3960dcc06de5a1218c43422f3ce4d8..1163b9af108a472c93b5d2901ff4767d1bd47356 100644 (file)
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-ce0674b1925138f8f878b11aae0f8420bd968df0959f6dd7e208fb84bcbad07e
\ No newline at end of file
+795f22421b96cea27b3883a580d8ce29cc50dfb7d4f852b3d36ede3766c7a24c
\ No newline at end of file