From: stephan <stephan@noemail.net>
Date: Wed, 9 Jul 2025 13:43:53 +0000 (+0000)
Subject: wasm: DB.wrapHandle() no longer accepts a DB object as its first argument, as there... 
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2ba5a93e8f6f42e617fad7b840d0720bb1dfbe26;p=thirdparty%2Fsqlite.git

wasm: DB.wrapHandle() no longer accepts a DB object as its first argument, as there's no apparent use case for proxying one DB object with another. Doc improvements for the new code.

FossilOrigin-Name: 0ee19db02a9b9320681d4393d9ba52889d149eac8ee114fc7b7f52a57271c139
---

diff --git a/ext/wasm/api/sqlite3-api-oo1.c-pp.js b/ext/wasm/api/sqlite3-api-oo1.c-pp.js
index cc4c9bb287..27176c0264 100644
--- a/ext/wasm/api/sqlite3-api-oo1.c-pp.js
+++ b/ext/wasm/api/sqlite3-api-oo1.c-pp.js
@@ -38,13 +38,15 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
   */
   const __ptrMap = new WeakMap();
   /**
-     A Set of oo1.DB objects which are proxies for (A) (sqlite3*) or
-     another oo1.DB object or (B) oo1.Stmt objects which are proxies
-     for (sqlite3_stmt*) pointers. Such objects do not own their
-     underlying handle and that handle must be guaranteed (by the
-     client) to outlive the proxy. These proxies are primarily
-     intended as a way to briefly wrap an (sqlite3[_stmt]*) object as
-     an oo1.DB/Stmt without taking over ownership.
+     A Set of oo1.DB or oo1.Stmt objects which are proxies for
+     (sqlite3*) resp. (sqlite3_stmt*) pointers. Such objects
+     optionally do not own their underlying handle and that handle
+     must be guaranteed (by the client) to outlive the proxy. These
+     proxies are primarily intended as a way to briefly wrap an
+     (sqlite3[_stmt]*) object as an oo1.DB/Stmt without taking over
+     ownership.
+
+     See DB.wrapHandle() and Stmt.wrapHandle().
   */
   const __doesNotOwnHandle = new Set();
   /**
@@ -1488,22 +1490,17 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
   }/*DB.prototype*/;
 
   /**
-     Returns a new oo1.DB instance which wraps the given db.
+     Returns a new oo1.DB instance which wraps the given (sqlite3*)
+     WASM pointer, optionally with or without taking over ownership of
+     that pointer.
 
      The first argument must be either a non-NULL (sqlite3*) WASM
-     pointer or a non-close()d instance of oo1.DB.
-
-     The second argument, defaulting to false, only applies if the
-     first argument is a (sqlite3*). If it is, the returned object
-     will pass that pointer to sqlite3_close() when its close() method
-     is called, otherwise it will not.
+     pointer.
 
-     If the first argument is a oo1.DB object, the second argument is
-     disregarded and the returned object will be created as a
-     sqlite3.oo1.DB object (as opposed to the concrete derived DB
-     subclass from the first argument), so will not include any
-     derived-type behaviors,
-     e.g. JsStorageDb.prototype.clearStorage().
+     The second argument, defaulting to false, specifies ownership of
+     the first argument. If it is truthy, the returned object will
+     pass that pointer to sqlite3_close() when its close() method is
+     called, otherwise it will not.
 
      Throws if db cannot be resolved to one of the legal options.
 
@@ -1515,56 +1512,38 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
      DB instance, including the requirement of calling close() on
      it. close() will free up internal resources owned by the proxy,
      and disassociate the proxy from that handle, but will not
-     actually close the proxied db handle.
+     actually close the proxied db handle unless this function is
+     passed a thruthy second argument.
 
      The following quirks and requirements apply when proxying another
      DB instance, as opposed to a (sqlite3*):
 
-     - DO NOT call close() on the being-proxied instance while a proxy
-       is active.
-
-     - ALWAYS eventually call close() on the returned object BEFORE
-       the being-proxied handle is closed.
-
-    - For historical reasons, the filename property of the returned
-      object is captured at the time of this call, as opposed to being
-      dynamically proxied. e.g., if the filename property of the
-      being-proxied object is changed, this object will not reflect
-      that change. There is no good reason to ever modify that
-      property, so this distinction is not truly significant but it's
-      noted here because it's a client-visible discrepancy between the
-      proxy and its partner. (Sidebar: the filename property _should_
-      be a property access interceptor for sqlite3_db_filename(),
-      but making it so now may break existing code.)
+     - DO NOT call sqlite3_close() (or similar) on the being-proxied
+       instance while a proxy is active.
+
+     - ALWAYS eventually call close() on the returned object. If the
+       proxy does not own the underlying handle then its MUST be
+       closed BEFORE the being-proxied handle is closed.
+
+     Design notes:
+
+     - wrapHandle() "could" accept a DB object instance as its first
+       argument and proxy thatDb.pointer but there is currently no use
+       case where doing so would be useful, so it does not allow
+       that. That restriction may be lifted in a future version.
   */
-  DB.wrapHandle = function(db, takeOwnership=false){
-    let ptr, ctor = DB;
-    const oo1db = (db instanceof DB) ? db : undefined;
-    if( wasm.isPtr(db) ){
-      ptr = db;
-    }else if( oo1db ){
-      takeOwnership = false;
-      ptr = db.pointer;
-      //ctor = db.constructor;
-      // ^^^ that doesn't work, resulting in an Object-type value
-    }
-    //sqlite3.config.debug("wrapHandle()",'db',db,'ctor',ctor,
-    //'arguments',arguments,'db.constructor',db.constructor);
-    if( !ptr ){
-      throw new sqlite3.SQLite3Error(sqlite3.SQLITE_MISUSE,
-                                     "Argument must be a WASM sqlite3 "+
-                                     "pointer or an sqlite3.oo1.DB instance");
+  DB.wrapHandle = function(pDb, takeOwnership=false){
+    if( !pDb || !wasm.isPtr(pDb) ){
+      throw new sqlite3.SQLite3Error(capi.SQLITE_MISUSE,
+                                     "Argument must be a WASM sqlite3 pointer");
     }
-    const dc = new ctor({
-      "sqlite3*": ptr,
+    return new DB({
+      /* This ctor call style is very specifically internal-use-only.
+         It is not documented and may change at any time. */
+      "sqlite3*": pDb,
       "sqlite3*:takeOwnership": !!takeOwnership
     });
-    if( oo1db ){
-      dc.filename = oo1db.filename;
-    }//else dc.filename was captured by the ctor for legacy consistency
-    //sqlite3.config.debug("wrapHandle() dc",dc);
-    return dc;
-  }/*DB.wrapHandle()*/;
+  };
 
   /** Throws if the given Stmt has been finalized, else stmt is
       returned. */
@@ -2260,8 +2239,8 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
 
   /**
      The Stmt counterpart of oo1.DB.wrapHandle(), this creates a Stmt
-     instance which wraps a WASM (sqlite3_stmt*) in the oo1 API with
-     or without taking over ownership of that pointer.
+     instance which wraps a WASM (sqlite3_stmt*) in the oo1 API,
+     optionally with or without taking over ownership of that pointer.
 
      The first argument must be an oo1.DB instance[^1].
 
@@ -2272,7 +2251,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
      returned Stmt object takes over ownership of the underlying
      (sqlite3_stmt*). If true, the returned object's finalize() method
      will finalize that handle, else it will not. If it is false,
-     ownership of stmtPtr is unchanged and stmtPtr MUST outlive the
+     ownership of pStmt is unchanged and pStmt MUST outlive the
      returned object or results are undefined.
 
      This function throws if the arguments are invalid. On success it
@@ -2289,21 +2268,25 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
      determined whether it would be of general benefit to refactor the
      DB/Stmt pair internals to communicate in terms of the underlying
      (sqlite3*) rather than a DB object. If so, we could laxen the
-     first argument's requirement and allow an (sqlite3*).
+     first argument's requirement and allow an (sqlite3*). Because
+     DB.wrapHandle() enables multiple DB objects to proxy the same
+     (sqlite3*), we cannot unambiguously translate the first arugment
+     from (sqlite3*) to DB instances for us with this function's first
+     argument.
   */
-  Stmt.wrapHandle = function(oo1db, stmtPtr, takeOwnership=false){
+  Stmt.wrapHandle = function(oo1db, pStmt, takeOwnership=false){
     let ctor = Stmt;
     if( !(oo1db instanceof DB) || !oo1db.pointer ){
       throw new sqlite3.SQLite3Error(sqlite3.SQLITE_MISUSE,
                                      "First argument must be an opened "+
                                      "sqlite3.oo1.DB instance");
     }
-    if( !stmtPtr || !wasm.isPtr(stmtPtr) ){
+    if( !pStmt || !wasm.isPtr(pStmt) ){
       throw new sqlite3.SQLite3Error(sqlite3.SQLITE_MISUSE,
                                      "Second argument must be a WASM "+
                                      "sqlite3_stmt pointer");
     }
-    return new Stmt(oo1db, stmtPtr, BindTypes, !!takeOwnership);
+    return new Stmt(oo1db, pStmt, BindTypes, !!takeOwnership);
   }
 
   /** The OO API's public namespace. */
diff --git a/ext/wasm/tester1.c-pp.js b/ext/wasm/tester1.c-pp.js
index aafe4d7f3f..f2fdeebed6 100644
--- a/ext/wasm/tester1.c-pp.js
+++ b/ext/wasm/tester1.c-pp.js
@@ -221,7 +221,7 @@ globalThis.sqlite3InitModule = sqlite3InitModule;
       else if(filter instanceof Function) pass = filter(err);
       else if('string' === typeof filter) pass = (err.message === filter);
       if(!pass){
-        throw new Error(msg || ("Filter rejected this exception: "+err.message));
+        throw new Error(msg || ("Filter rejected this exception: <<"+err.message+">>"));
       }
       return this;
     },
@@ -1219,7 +1219,10 @@ globalThis.sqlite3InitModule = sqlite3InitModule;
            function do not interfere with downstream tests, e.g. by
            closing this.db.pointer. */
         //sqlite3.config.debug("Proxying",this.db);
-        let dw = sqlite3.oo1.DB.wrapHandle(this.db);
+        const misuseMsg = "SQLITE_MISUSE: Argument must be a WASM sqlite3 pointer";
+        T.mustThrowMatching(()=>sqlite3.oo1.DB.wrapHandle(this.db), misuseMsg)
+          .mustThrowMatching(()=>sqlite3.oo1.DB.wrapHandle(0), misuseMsg);
+        let dw = sqlite3.oo1.DB.wrapHandle(this.db.pointer);
         //sqlite3.config.debug('dw',dw);
         T.assert( dw, '!!dw' )
           .assert( dw instanceof sqlite3.oo1.DB, 'dw is-a oo1.DB' )
diff --git a/manifest b/manifest
index da6bf18ef5..abeec3b372 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C wasm:\sadd\sa\sfew\stests\sdemonstrating\sthat\soo1.Stmt.paramaterCount's\snew\simpl\sdoes\snot\schange\svisible\sbehaviors.\sAdd\sthe\sc-pp-filtered\sfiles\sto\s'make\sclean'.
-D 2025-07-09T13:13:01.736
+C wasm:\sDB.wrapHandle()\sno\slonger\saccepts\sa\sDB\sobject\sas\sits\sfirst\sargument,\sas\sthere's\sno\sapparent\suse\scase\sfor\sproxying\sone\sDB\sobject\swith\sanother.\sDoc\simprovements\sfor\sthe\snew\scode.
+D 2025-07-09T13:43:53.471
 F .fossil-settings/binary-glob 61195414528fb3ea9693577e1980230d78a1f8b0a54c78cf1b9b24d0a409ed6a x
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
@@ -641,7 +641,7 @@ F ext/wasm/api/post-js-header.js 53740d824e5d9027eb1e6fd59e216abbd2136740ce260ea
 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 0b76510f3650053bac67ca8947cb6ab9d050ad2218118a2e7796dd37be832ffa
-F ext/wasm/api/sqlite3-api-oo1.c-pp.js 33203352e067d3dbb569c5ec3c247b356f61a8d320bf8551fdf18ff25c0ff862
+F ext/wasm/api/sqlite3-api-oo1.c-pp.js 972e0aa2f6f3ad9d9530edfc5c18a56e55ab5bec728ce6ca9b89ce1df463f740
 F ext/wasm/api/sqlite3-api-prologue.js 8708570165f5b4bce9a78ccd91bc9ddf8735970ac1c4d659e36c9a7d9a644bb4
 F ext/wasm/api/sqlite3-api-worker1.c-pp.js f646a65257973b8c4481f8a6a216370b85644f23e64b126e7ae113570587c0ab
 F ext/wasm/api/sqlite3-license-version-header.js 0c807a421f0187e778dc1078f10d2994b915123c1223fe752b60afdcd1263f89
@@ -698,7 +698,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 1c4fdb32e10c92e2595939b1b6f6588cd3a30a2691ba2dc4a42893db99e0648b
+F ext/wasm/tester1.c-pp.js 830749ee21bde538f52e150878c0078a3f6d294ba183900efd8488a9e6debf4f
 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
@@ -2208,8 +2208,8 @@ F tool/version-info.c 3b36468a90faf1bbd59c65fd0eb66522d9f941eedd364fabccd7227350
 F tool/warnings-clang.sh bbf6a1e685e534c92ec2bfba5b1745f34fb6f0bc2a362850723a9ee87c1b31a7
 F tool/warnings.sh 1ad0169b022b280bcaaf94a7fa231591be96b514230ab5c98fbf15cd7df842dd
 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f
-P 6e73770a7f3845055e0130012d844c32c4a1bfdb87e8379c161e1a266a808143
-R 275bf35e86e01e823d3a78eac9fe64ed
+P 3fe61545967f82190011edb90b1be6a448b590555c7ba5f8e96494aeea8f88ce
+R 7d3270f820abddff176885a6dceb6626
 U stephan
-Z 5ade71be55c22a5952ee210feb5c3747
+Z 4ed976109c5d88084cce8c9c03b32258
 # Remove this line to create a well-formed Fossil manifest.
diff --git a/manifest.uuid b/manifest.uuid
index dbc806e8f1..b2ae9d924e 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-3fe61545967f82190011edb90b1be6a448b590555c7ba5f8e96494aeea8f88ce
+0ee19db02a9b9320681d4393d9ba52889d149eac8ee114fc7b7f52a57271c139