Add a more complete test for [76c8435a] and add some commentary about (A) the inability to automatically clean up automatically-generated WASM proxy functions for sqlite3_set_auxdata() destructors and (B) how to deal with (A) to avoid leaking WASM proxy functions.

FossilOrigin-Name: d693c2dddbd10a2e0b77893b04b11502e30b768f1b06814105f7f35172845fb9

4 files changed, 105 insertions, 10 deletions

diff --git a/ext/wasm/api/sqlite3-api-glue.c-pp.js b/ext/wasm/api/sqlite3-api-glue.c-pp.js
index ddcf2535f..bcaff7243 100644
--- a/ext/wasm/api/sqlite3-api-glue.c-pp.js
+++ b/ext/wasm/api/sqlite3-api-glue.c-pp.js
@@ -228,6 +228,31 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
       }),
       '*'
     ]],
+    /**
+       2025-02-03: We do not have a way to automatically clean up
+       destructors which are automatically converted from JS functions
+       via the final argument to sqlite3_set_auxdata(). Because of
+       that, it is strongly recommended that clients use
+       wasm.installFunction() to create such callbacks, then pass that
+       pointer to sqlite3_set_auxdata(). Relying on automated
+       conversions here will lead to leaks of JS/WASM proxy functions
+       because sqlite3_set_auxdata() is frequently called in UDFs.
+
+       The sqlite3.oo1.DB class's onclose handlers can be used for this
+       purpose. For example:
+
+       const pAuxDtor = wasm.installFunction('v(p)', function(ptr){
+         //free ptr
+       });
+       myDb.onclose = {
+         after: ()=>{
+           wasm.uninstallFunction(pAuxDtor);
+         }
+       };
+
+       Then pass pAuxDtor as the final argument to appropriate
+       sqlite3_set_auxdata() calls.
+    */
     ["sqlite3_set_auxdata", undefined, [
       "sqlite3_context*", "int", "*",
       new wasm.xWrap.FuncPtrAdapter({
@@ -1047,6 +1072,10 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
       'sqlite3_set_authorizer',
       'sqlite3_trace_v2',
       'sqlite3_update_hook'
+      /*
+        We do not yet have a way to clean up automatically-converted
+        sqlite3_set_auxdata() finalizers.
+      */
     ]) {
       const x = wasm.exports[name];
       if( !x ){
diff --git a/ext/wasm/tester1.c-pp.js b/ext/wasm/tester1.c-pp.js
index f83ecbd87..880edcec1 100644
--- a/ext/wasm/tester1.c-pp.js
+++ b/ext/wasm/tester1.c-pp.js
@@ -3437,6 +3437,73 @@ globalThis.sqlite3InitModule = sqlite3InitModule;
         }
       }
     })
+    .t({
+      /* https://github.com/sqlite/sqlite-wasm/issues/92 */
+      name: 'sqlite3_set_auxdata() binding signature',
+      test: function(sqlite3){
+        const db = new sqlite3.oo1.DB();
+        const stack = wasm.pstack.pointer;
+        const pAux = wasm.pstack.alloc(4);
+        let pAuxDestructed = 0;
+        const args = [];
+        const pAuxDtor = wasm.installFunction('v(p)', function(ptr){
+          //log("freeing auxdata");
+          ++pAuxDestructed;
+        });
+        let pAuxDtorDestructed = false;
+        db.onclose = {
+          after: ()=>{
+            pAuxDtorDestructed = true;
+            wasm.uninstallFunction(pAuxDtor);
+          }
+        };
+        try{
+          db.createFunction("auxtest",{
+            xFunc: function(pCx, x, y){
+              args.push(x);
+              T.assert(wasm.isPtr(pCx));
+              const localAux = capi.sqlite3_get_auxdata(pCx, 0);
+              if( !localAux ){
+                //log("setting auxdata");
+                /**
+                   We do not currently an automated way to clean up
+                   auxdata finalizer functions (the 4th argument to
+                   sqlite3_set_auxdata()) which get automatically
+                   converted from JS to WASM. Because of that, relying
+                   on automated conversions for those is not
+                   recommended. Instead, follow the pattern show in
+                   this function: use wasm.installFunction() to create
+                   the function, then pass the resulting function
+                   pointer this function, and cleanup (at some point)
+                   using wasm.uninstallFunction().
+                */
+                capi.sqlite3_set_auxdata(pCx, 0, pAux, pAuxDtor);
+              }else{
+                /* This is never actually hit in this example and it's
+                   not entirely clear how to cause it to. The point of
+                   this test, however, is to demonstrate that the
+                   finalizer impl gets triggered, so we're not going to
+                   fret over this at the moment. */
+                //log("seen auxdata",localAux);
+                T.assert(pAux===localAux);
+              }
+              return x;
+            }
+          });
+          db.exec([
+            "create table t(a);",
+            "insert into t(a) values(1),(2),(3);",
+            "select auxtest(a,a), auxtest(a,a) from t order by a"
+          ]);
+        }finally{
+          db.close();
+          wasm.pstack.restore(stack);
+        }
+        T.assert(6===args.length);
+        T.assert(pAuxDestructed>0);
+        T.assert(pAuxDtorDestructed);
+      }
+    })
   ;/*end of Bug Reports group*/;
 
   ////////////////////////////////////////////////////////////////////////
diff --git a/manifest b/manifest
index c36f2724d..54fc8aa8a 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Fix\sthe\sbuild\sprocess\son\sWindows\sso\sthat\sit\sgenerates\sidentical\ssqlite3.c,\nsqlite3.h,\sand\sshell.c\sfiles\son\sWindows\sand\sUnix.\s\sThis\spatch\salso\sincludes\na\schange\sto\sJS\sbindings\sthat\sgot\scaught\sup\sin\sthe\sbranch.
-D 2025-02-03T14:44:16.557
+C Add\sa\smore\scomplete\stest\sfor\s[76c8435a]\sand\sadd\ssome\scommentary\sabout\s(A)\sthe\sinability\sto\sautomatically\sclean\sup\sautomatically-generated\sWASM\sproxy\sfunctions\sfor\ssqlite3_set_auxdata()\sdestructors\sand\s(B)\show\sto\sdeal\swith\s(A)\sto\savoid\sleaking\sWASM\sproxy\sfunctions.
+D 2025-02-03T14:55:56.185
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md e108e1e69ae8e8a59e93c455654b8ac9356a11720d3345df2a4743e9590fb20d
@@ -638,7 +638,7 @@ F ext/wasm/api/post-js-footer.js 365405929f41ca0e6d389ed8a8da3f3c93e11d3ef43a90a
 F ext/wasm/api/post-js-header.js 54b2b4294501b3866245cc94315a16f5424c0e87729d0fb610fba151593c6d26
 F ext/wasm/api/pre-js.c-pp.js a614a2c82b12c4d96d8e3ba77330329efc53c4d56a8a7e60ade900f341866cfb
 F ext/wasm/api/sqlite3-api-cleanup.js 3ac1786e461ada63033143be8c3b00b26b939540661f3e839515bb92f2e35359
-F ext/wasm/api/sqlite3-api-glue.c-pp.js 0fc6a377907a5101ee426fda7e91def6a310785b23b7a39dd8b2c5e47ee36b4b
+F ext/wasm/api/sqlite3-api-glue.c-pp.js 6e2f2eaf681e342fcb047fcdd01d6e3c1b466fb9b45c1acc38676164a8b60f45
 F ext/wasm/api/sqlite3-api-oo1.c-pp.js f3a8e2004c6625d17946c11f2fb32008be78bc5207bf746fc77d59848813225f
 F ext/wasm/api/sqlite3-api-prologue.js 5ff913355b3144f1c9719d0406667fa6e13eb813c71ed7ce29440e2e65363e82
 F ext/wasm/api/sqlite3-api-worker1.c-pp.js 5cc22a3c0d52828cb32aad8691488719f47d27567e63e8bc8b832d74371c352d
@@ -696,7 +696,7 @@ F ext/wasm/test-opfs-vfs.html 1f2d672f3f3fce810dfd48a8d56914aba22e45c6834e262555
 F ext/wasm/test-opfs-vfs.js 1618670e466f424aa289859fe0ec8ded223e42e9e69b5c851f809baaaca1a00c
 F ext/wasm/tester1-worker.html ebc4b820a128963afce328ecf63ab200bd923309eb939f4110510ab449e9814c
 F ext/wasm/tester1.c-pp.html 1c1bc78b858af2019e663b1a31e76657b73dc24bede28ca92fbe917c3a972af2
-F ext/wasm/tester1.c-pp.js 05df7cfd2cb8aa8728afb68c90a8af51fdf724c0e892f0f986a695584edae195
+F ext/wasm/tester1.c-pp.js fb8d0761daaa69bd40c8253cc2d6c8c37ada97e1751b7f07af7369842ba2aeae
 F ext/wasm/tests/opfs/concurrency/index.html 657578a6e9ce1e9b8be951549ed93a6a471f4520a99e5b545928668f4285fb5e
 F ext/wasm/tests/opfs/concurrency/test.js d08889a5bb6e61937d0b8cbb78c9efbefbf65ad09f510589c779b7cc6a803a88
 F ext/wasm/tests/opfs/concurrency/worker.js 0a8c1a3e6ebb38aabbee24f122693f1fb29d599948915c76906681bb7da1d3d2
@@ -2209,9 +2209,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 602d4dd69ec9a724c69cb41ab15376ec731bfd4894fac0a2b25076b857786c6d 52a7a162d131532de8f4403b099fa122fc72c80f71494fb561afc4e0ff8e6bf4
-R ffc4ba900908f24af2ec6f8d798eed37
-T +closed 52a7a162d131532de8f4403b099fa122fc72c80f71494fb561afc4e0ff8e6bf4
-U drh
-Z 167889892c0bdca97b0a614e610568d1
+P 91ef45fc2902e46813366ec6b8317209f39f10e4a23c3808e33aceedab9da6c7
+R f2c90877762eddda2efda339cfb2ee34
+U stephan
+Z 6cfffa02c18a4e52a298c977368cc8d7
 # Remove this line to create a well-formed Fossil manifest.
diff --git a/manifest.uuid b/manifest.uuid
index 367348a3f..c7225ebda 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-91ef45fc2902e46813366ec6b8317209f39f10e4a23c3808e33aceedab9da6c7
+d693c2dddbd10a2e0b77893b04b11502e30b768f1b06814105f7f35172845fb9