Reimplement sqlite3.capi.sqlite3_close_v2() and sqlite3session_delete() as a hand-written bindings so that they can attempt to clean up certain (potentially) FuncPtrAdapter-installed functions before closing. Correct the create-function family of JS-to-function-pointer automated conversions to include the UDF's arity as part of the mapping's key so that (un)binding a UDF to different functions for different arities works (and add tests confirming it). Correct a broken doc link in module-symbols.html.

FossilOrigin-Name: 60b262ef0f57b162c2566b12e70685a92afb00b441332ea7a6540fcb188cc7af

4 files changed, 135 insertions, 11 deletions

diff --git a/ext/wasm/api/sqlite3-api-glue.js b/ext/wasm/api/sqlite3-api-glue.js
index 8a0d8147f..94554e0b5 100644
--- a/ext/wasm/api/sqlite3-api-glue.js
+++ b/ext/wasm/api/sqlite3-api-glue.js
@@ -87,7 +87,8 @@ self.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
       "*"
     ]],
     ["sqlite3_busy_timeout","int", "sqlite3*", "int"],
-    ["sqlite3_close_v2", "int", "sqlite3*"],
+    /*[sqlite3_close_v2() is implemented by hand to perform some
+       extra work. "sqlite3_close_v2", "int", "sqlite3*"],*/
     ["sqlite3_changes", "int", "sqlite3*"],
     ["sqlite3_clear_bindings","int", "sqlite3_stmt*"],
     ["sqlite3_collation_needed", "int", "sqlite3*", "*", "*"/*=>v(ppis)*/],
@@ -489,7 +490,7 @@ self.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
       ]],
       ['sqlite3session_config', 'int', ['int', 'void*']],
       ['sqlite3session_create', 'int', ['sqlite3*', 'string', '**']],
-      ['sqlite3session_delete', undefined, ['sqlite3_session*']],
+      //sqlite3session_delete() is bound manually
       ['sqlite3session_diff', 'int', ['sqlite3_session*', 'string', 'string', '**']],
       ['sqlite3session_enable', 'int', ['sqlite3_session*', 'int']],
       ['sqlite3session_indirect', 'int', ['sqlite3_session*', 'int']],
@@ -730,6 +731,67 @@ self.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
     );
   };
 
+  {/* Binding of sqlite3_close_v2() */
+    const __sqlite3CloseV2 = wasm.xWrap("sqlite3_close_v2", "int", "sqlite3*");
+    capi.sqlite3_close_v2 = function(pDb){
+      if(1!==arguments.length) return __dbArgcMismatch(pDb, 'sqlite3_close_v2', 1);
+      if(pDb){
+        /*
+          We do this as a basic attempt at freeing up certain
+          automatically-installed WASM function bindings, as those may
+          otherwise leak. Installing NULL functions in the C API will
+          remove those bindings. The FuncPtrAdapter which sits between
+          us and the C API will also treat that as an opportunity to
+          wasm.uninstallFunction() any WASM function bindings it has
+          installed for pDb.
+
+          This does not catch all such bindings: those which map to
+          both a db handle and a separate key (e.g. collation sequence
+          name or UDF name) cannot be unmapped here because we don't
+          have the other parts of the mapping key. It's also possible
+          for clients to call wasm.exports.sqlite3_close_v2()
+          directly, bypassing this cleanup altogether. i.e. this is
+          not a silver bullet, just an "honest effort."
+
+          Perhaps we can add some code to sqlite3-wasm.c which can
+          walk through the UDF and collation names to help us free up
+          those auto-converted functions, too. Functions are more
+          complicated because a given function may have multiple
+          mappings for different arities.
+
+          The issue being addressed here is covered at:
+
+          https://sqlite.org/wasm/doc/trunk/api-c-style.md#convert-func-ptr
+        */
+        //wasm.xWrap.FuncPtrAdapter.debugFuncInstall = true;
+        try{capi.sqlite3_busy_handler(pDb, 0, 0)} catch(e){/*ignored*/}
+        try{capi.sqlite3_progress_handler(pDb, 0, 0, 0)} catch(e){/*ignored*/}
+        try{capi.sqlite3_trace_v2(pDb, 0, 0, 0, 0)} catch(e){/*ignored*/}
+        try{capi.sqlite3_set_authorizer(pDb, 0, 0)} catch(e){/*ignored*/}
+      }
+      return __sqlite3CloseV2(pDb);
+    };
+  }/*sqlite3_close_v2()*/
+
+  if(capi.sqlite3session_table_filter){
+    const __sqlite3SessionDelete = wasm.xWrap(
+      'sqlite3session_delete', undefined, ['sqlite3_session*']
+    );
+    capi.sqlite3session_delete = function(pSession){
+      if(1!==arguments.length){
+        return __dbArgcMismatch(pDb, 'sqlite3session_delete', 1);
+        /* Yes, we're returning a value from a void function. That seems
+           like the lesser evil compared to not maintaining arg-count
+           consistency as we do with other similar bindings. */
+      }
+      else if(pSession){
+        //wasm.xWrap.FuncPtrAdapter.debugFuncInstall = true;
+        capi.sqlite3session_table_filter(pSession, 0, 0);
+      }
+      __sqlite3SessionDelete(pSession);
+    };
+  }
+
   {/* Bindings for sqlite3_create_collation[_v2]() */
     // contextKey() impl for wasm.xWrap.FuncPtrAdapter
     const contextKey = (argv,argIndex)=>{
@@ -798,13 +860,13 @@ self.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
 
   {/* Special-case handling of sqlite3_create_function_v2()
       and sqlite3_create_window_function(). */
-    /**
-       FuncPtrAdapter for contextKey() for sqlite3_create_function().
-    */
+    /** FuncPtrAdapter for contextKey() for sqlite3_create_function()
+        and friends. */
     const contextKey = function(argv,argIndex){
       return (
         argv[0/* sqlite3* */]
-          +':'+argIndex
+          +':'+(argv[2/*number of UDF args*/] < 0 ? -1 : argv[2])
+          +':'+argIndex/*distinct for each xAbc callback type*/
           +':'+wasm.cstrToJs(argv[1]).toLowerCase()
       )
     };
diff --git a/ext/wasm/common/whwasmutil.js b/ext/wasm/common/whwasmutil.js
index 9f634d774..df401c8fe 100644
--- a/ext/wasm/common/whwasmutil.js
+++ b/ext/wasm/common/whwasmutil.js
@@ -1499,7 +1499,7 @@ self.WhWasmUtilInstaller = function(target){
   */
   const AbstractArgAdapter = class {
     constructor(opt){
-      this.name = opt.name;
+      this.name = opt.name || 'unnamed adapter';
     }
     /**
        Gets called via xWrap() to "convert" v to whatever type
@@ -1651,8 +1651,21 @@ self.WhWasmUtilInstaller = function(target){
         ? opt.callProxy : undefined;
     }
 
+    /** If true, the constructor emits a warning. The intent is that
+        this be set to true after bootstrapping of the higher-level
+        client library is complete, to warn downstream clients that
+        they shouldn't be relying on this implemenation detail which
+        does not have a stable interface. */
     static warnOnUse = false;
 
+    /** If true, convertArg() will FuncPtrAdapter.debugOut() when it
+        (un)installs a function binding to/from WASM.
+    */
+    static debugFuncInstall = false;
+
+    /** Function used for debug output. */
+    static debugOut = console.debug.bind(console);
+
     static bindScopes = [
       'transient', 'context', 'singleton'
     ];
@@ -1692,7 +1705,7 @@ self.WhWasmUtilInstaller = function(target){
        exactly the 2nd and 3rd arguments are.
     */
     convertArg(v,argv,argIndex){
-      //console.warn("FuncPtrAdapter.convertArg()",this.signature,this.transient,v);
+      //FuncPtrAdapter.debugOut("FuncPtrAdapter.convertArg()",this.signature,this.transient,v);
       let pair = this.singleton;
       if(!pair && this.isContext){
         pair = this.contextMap(this.contextKey(argv,argIndex));
@@ -1702,9 +1715,17 @@ self.WhWasmUtilInstaller = function(target){
         /* Install a WASM binding and return its pointer. */
         if(this.callProxy) v = this.callProxy(v);
         const fp = __installFunction(v, this.signature, this.isTransient);
+        if(FuncPtrAdapter.debugFuncInstall){
+          FuncPtrAdapter.debugOut("FuncPtrAdapter installed", this,
+                                  this.contextKey(argv,argIndex), '@'+fp, v);
+        }
         if(pair){
           /* Replace existing stashed mapping */
           if(pair[1]){
+            if(FuncPtrAdapter.debugFuncInstall){
+              FuncPtrAdapter.debugOut("FuncPtrAdapter uninstalling", this,
+                                      this.contextKey(argv,argIndex), '@'+pair[1], v);
+            }
             try{target.uninstallFunction(pair[1])}
             catch(e){/*ignored*/}
           }
@@ -1715,7 +1736,10 @@ self.WhWasmUtilInstaller = function(target){
       }else if(target.isPtr(v) || null===v || undefined===v){
         if(pair && pair[1] && pair[1]!==v){
           /* uninstall stashed mapping and replace stashed mapping with v. */
-          //console.warn("FuncPtrAdapter is uninstalling function", this.contextKey(argv,argIndex),v);
+          if(FuncPtrAdapter.debugFuncInstall){
+            FuncPtrAdapter.debugOut("FuncPtrAdapter uninstalling", this,
+                                    this.contextKey(argv,argIndex), '@'+pair[1], v);
+          }
           try{target.uninstallFunction(pair[1])}
           catch(e){/*ignored*/}
           pair[0] = pair[1] = (v | 0);
diff --git a/ext/wasm/module-symbols.html b/ext/wasm/module-symbols.html
index 85fd3ebb3..ff9c9769d 100644
--- a/ext/wasm/module-symbols.html
+++ b/ext/wasm/module-symbols.html
@@ -290,7 +290,7 @@
         sqlite3_result_zeroblob64: 'www:/c3ref/result_blob.html',
         sqlite3_serialize: 'www:/c3ref/serialize.html',
         sqlite3_set_authorizer: 'wasm:/api-c-style.md#sqlite3_set_authorizer',
-        sqlite3_set_auxdata: 'www:/c3ref/set_auxdata.html',
+        sqlite3_set_auxdata: 'www:/c3ref/get_auxdata.html',
         sqlite3_set_last_insert_rowid: 'www:/c3ref/set_last_insert_rowid',
         sqlite3_shutdown: 'www:/c3ref/initialize.html',
         sqlite3_sourceid: 'www:/c3ref/libversion.html',
diff --git a/ext/wasm/tester1.c-pp.js b/ext/wasm/tester1.c-pp.js
index bf4497a3e..0c15bc063 100644
--- a/ext/wasm/tester1.c-pp.js
+++ b/ext/wasm/tester1.c-pp.js
@@ -1629,7 +1629,6 @@ self.sqlite3InitModule = sqlite3InitModule;
   ////////////////////////////////////////////////////////////////////
     .t({
       name:'Scalar UDFs',
-      //predicate: ()=>false,
       test: function(sqlite3){
         const db = this.db;
         db.createFunction("foo",(pCx,a,b)=>a+b);
@@ -1696,6 +1695,45 @@ self.sqlite3InitModule = sqlite3InitModule;
           sqlite3.capi.SQLITE_MISUSE === rc,
           "For invalid arg count."
         );
+
+        /* Confirm that we can map and unmap the same function with
+           multiple arities... */
+        const fCounts = [0,0];
+        const fArityCheck = function(pCx){
+          return ++fCounts[arguments.length-1];
+        };
+        //wasm.xWrap.FuncPtrAdapter.debugFuncInstall = true;
+        rc = capi.sqlite3_create_function_v2(
+          db, "nary", 0, capi.SQLITE_UTF8, 0, fArityCheck, 0, 0, 0
+        );
+        T.assert( 0===rc );
+        rc = capi.sqlite3_create_function_v2(
+          db, "nary", 1, capi.SQLITE_UTF8, 0, fArityCheck, 0, 0, 0
+        );
+        T.assert( 0===rc );
+        const sqlFArity0 = "select nary()";
+        const sqlFArity1 = "select nary(1)";
+        T.assert( 1 === db.selectValue(sqlFArity0) )
+          .assert( 1 === fCounts[0] ).assert( 0 === fCounts[1] );
+        T.assert( 1 === db.selectValue(sqlFArity1) )
+          .assert( 1 === fCounts[0] ).assert( 1 === fCounts[1] );
+        capi.sqlite3_create_function_v2(
+          db, "nary", 0, capi.SQLITE_UTF8, 0, 0, 0, 0, 0
+        );
+        T.mustThrowMatching((()=>db.selectValue(sqlFArity0)),
+                            (e)=>((e instanceof sqlite3.SQLite3Error)
+                                  && e.message.indexOf("wrong number of arguments")>0),
+                            "0-arity variant was uninstalled.");
+        T.assert( 2 === db.selectValue(sqlFArity1) )
+          .assert( 1 === fCounts[0] ).assert( 2 === fCounts[1] );
+        capi.sqlite3_create_function_v2(
+          db, "nary", 1, capi.SQLITE_UTF8, 0, 0, 0, 0, 0
+        );
+        T.mustThrowMatching((()=>db.selectValue(sqlFArity1)),
+                            (e)=>((e instanceof sqlite3.SQLite3Error)
+                                  && e.message.indexOf("no such function")>0),
+                            "1-arity variant was uninstalled.");
+        //wasm.xWrap.FuncPtrAdapter.debugFuncInstall = false;
       }
     })