oo1.Stmt.finalize() no longer throws, but instead returns the same as the C API....

FossilOrigin-Name: 15f105c78268fb68a4713da444452f5d7b038019b21024302beebb6b26ff2244

diff --git a/ext/wasm/api/sqlite3-api-oo1.js b/ext/wasm/api/sqlite3-api-oo1.js
index d46ee8dd43c7621e812e24177e297ef198a0374b..4da629a6bcbdbe1e2f6a7a4b34740c7d087f3abc 100644 (file)
--- a/ext/wasm/api/sqlite3-api-oo1.js
+++ b/ext/wasm/api/sqlite3-api-oo1.js
@@ -554,7 +554,10 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
         }
         const pDb = this.pointer;
         Object.keys(__stmtMap.get(this)).forEach((k,s)=>{
-          if(s && s.pointer) s.finalize();
+          if(s && s.pointer){
+            try{s.finalize()}
+            catch(e){/*ignore*/}
+          }
         });
         __ptrMap.delete(this);
         __stmtMap.delete(this);
@@ -878,16 +881,16 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
                  and names. */) ? 0 : 1;
             evalFirstResult = false;
             if(arg.cbArg || resultRows){
-              for(; stmt.step(); stmt._isLocked = false){
+              for(; stmt.step(); stmt._lockedByExec = false){
                 if(0===gotColNames++) stmt.getColumnNames(opt.columnNames);
-                stmt._isLocked = true;
+                stmt._lockedByExec = true;
                 const row = arg.cbArg(stmt);
                 if(resultRows) resultRows.push(row);
                 if(callback && false === callback.call(opt, row, stmt)){
                   break;
                 }
               }
-              stmt._isLocked = false;
+              stmt._lockedByExec = false;
             }
             if(0===gotColNames){
               /* opt.columnNames was provided but we visited no result rows */
@@ -896,16 +899,20 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
           }else{
             stmt.step();
           }
-          stmt.finalize();
+          stmt.reset(
+            /* In order to trigger an exception in the
+               INSERT...RETURNING locking scenario:
+               https://sqlite.org/forum/forumpost/36f7a2e7494897df
+            */).finalize();
           stmt = null;
-        }
+        }/*prepare() loop*/
       }/*catch(e){
         sqlite3.config.warn("DB.exec() is propagating exception",opt,e);
         throw e;
       }*/finally{
         wasm.scopedAllocPop(stack);
         if(stmt){
-          delete stmt._isLocked;
+          delete stmt._lockedByExec;
           stmt.finalize();
         }
       }
@@ -1321,15 +1328,15 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
   };
 
   /**
-     If stmt._isLocked is truthy, this throws an exception
+     If stmt._lockedByExec is truthy, this throws an exception
      complaining that the 2nd argument (an operation name,
      e.g. "bind()") is not legal while the statement is "locked".
      Locking happens before an exec()-like callback is passed a
      statement, to ensure that the callback does not mutate or
      finalize the statement. If it does not throw, it returns stmt.
   */
-  const affirmUnlocked = function(stmt,currentOpName){
-    if(stmt._isLocked){
+  const affirmNotLockedByExec = function(stmt,currentOpName){
+    if(stmt._lockedByExec){
       toss3("Operation is illegal when statement is locked:",currentOpName);
     }
     return stmt;
@@ -1342,14 +1349,11 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
      success.
   */
   const bindOne = function f(stmt,ndx,bindType,val){
-    affirmUnlocked(affirmStmtOpen(stmt), 'bind()');
+    affirmNotLockedByExec(affirmStmtOpen(stmt), 'bind()');
     if(!f._){
       f._tooBigInt = (v)=>toss3(
         "BigInt value is too big to store without precision loss:", v
       );
-      /* Reminder: when not in BigInt mode, it's impossible for
-         JS to represent a number out of the range we can bind,
-         so we have no range checking. */
       f._ = {
         string: function(stmt, ndx, val, asBlob){
           const [pStr, n] = wasm.allocCString(val, true);
@@ -1423,33 +1427,28 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
 
   Stmt.prototype = {
     /**
-       "Finalizes" this statement. This is a no-op if the
-       statement has already been finalized. Returns
-       undefined. Most methods in this class will throw if called
-       after this is.
+       "Finalizes" this statement. This is a no-op if the statement
+       has already been finalized. Returns the result of
+       sqlite3_finalize() (0 on success, non-0 on error), or the
+       undefined value if the statement has already been
+       finalized. Regardless of success or failure, most methods in
+       this class will throw if called after this is.
 
        This method always throws if called when it is illegal to do
-       so, e.g. from a per-row callback handler of a DB.exec() call.
-
-       As of versions 3.42.1 and 3.43, this method will throw by
-       default if sqlite3_finalize() returns an error code, which can
-       happen in certain unusual cases involving locking. When it
-       throws for this reason, throwing is delayed until after all
-       resources are cleaned up. That is, the finalization still runs
-       to completion.
+       so. Namely, when triggered via a per-row callback handler of a
+       DB.exec() call.
     */
     finalize: function(){
       if(this.pointer){
-        affirmUnlocked(this,'finalize()');
-        const rc = capi.sqlite3_finalize(this.pointer),
-              db = this.db;
-        delete __stmtMap.get(db)[this.pointer];
+        affirmNotLockedByExec(this,'finalize()');
+        const rc = capi.sqlite3_finalize(this.pointer);
+        delete __stmtMap.get(this.db)[this.pointer];
         __ptrMap.delete(this);
         delete this._mayGet;
         delete this.parameterCount;
-        delete this._isLocked;
+        delete this._lockedByExec;
         delete this.db;
-        checkSqlite3Rc(db, rc);
+        return rc;
       }
     },
     /**
@@ -1459,7 +1458,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
        a DB.exec() call).
     */
     clearBindings: function(){
-      affirmUnlocked(affirmStmtOpen(this), 'clearBindings()')
+      affirmNotLockedByExec(affirmStmtOpen(this), 'clearBindings()')
       capi.sqlite3_clear_bindings(this.pointer);
       this._mayGet = false;
       return this;
@@ -1467,19 +1466,24 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
     /**
        Resets this statement so that it may be step()ed again from the
        beginning. Returns this object. Throws if this statement has
-       been finalized or if it may not legally be reset because it is
-       currently being used from a DB.exec() callback.
+       been finalized, if it may not legally be reset because it is
+       currently being used from a DB.exec() callback, or if the
+       underlying call to sqlite3_reset() returns non-0.
 
        If passed a truthy argument then this.clearBindings() is
        also called, otherwise any existing bindings, along with
        any memory allocated for them, are retained.
 
-       As of versions 3.42.1 and 3.43, this function throws if the
-       underlying call to sqlite3_reset() returns non-0. That is
-       necessary for catching errors in certain locking-related cases.
+       In verions 3.42.0 and earlier, this function did not throw if
+       sqlite3_reset() returns non-0, but it was discovered that
+       throwing (or significant extra client-side code) is necessary
+       in order to avoid certain silent failure scenarios, as
+       discussed at:
+
+       https://sqlite.org/forum/forumpost/36f7a2e7494897df
     */
     reset: function(alsoClearBinds){
-      affirmUnlocked(this,'reset()');
+      affirmNotLockedByExec(this,'reset()');
       if(alsoClearBinds) this.clearBindings();
       const rc = capi.sqlite3_reset(affirmStmtOpen(this).pointer);
       this._mayGet = false;
@@ -1632,7 +1636,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
        value is returned.  Throws on error.
     */
     step: function(){
-      affirmUnlocked(this, 'step()');
+      affirmNotLockedByExec(this, 'step()');
       const rc = capi.sqlite3_step(affirmStmtOpen(this).pointer);
       switch(rc){
           case capi.SQLITE_DONE: return this._mayGet = false;
@@ -1681,20 +1685,12 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){
        ```
     */
     stepFinalize: function(){
-      let rc, err;
       try{
-        rc = this.step();
-      }catch(e){
-        err = e;
-      }
-      if(err){
+        return this.step();
+      }finally{
         try{this.finalize()}
-        catch(x){/*ignored*/}
-        throw err;
-      }else{
-        this.finalize();
+        catch(e){/*ignored*/}
       }
-      return rc;
     },
     /**
        Fetches the value from the given 0-based column index of

diff --git a/ext/wasm/tester1.c-pp.js b/ext/wasm/tester1.c-pp.js
index e16b0f95c7e58fa88b74bf8e1331113f5574eef8..fc86b7f657eac47fe90f460a82ef8788d44fe25f 100644 (file)
--- a/ext/wasm/tester1.c-pp.js
+++ b/ext/wasm/tester1.c-pp.js
@@ -1220,6 +1220,7 @@ self.sqlite3InitModule = sqlite3InitModule;
       );
       //debug("statement =",st);
       this.progressHandlerCount = 0;
+      let rc;
       try {
         T.assert(wasm.isPtr(st.pointer))
           .mustThrowMatching(()=>st.pointer=1, /read-only/)
@@ -1267,10 +1268,11 @@ self.sqlite3InitModule = sqlite3InitModule;
           assert(0===capi.sqlite3_strlike("%.txt", "foo.txt", 0)).
           assert(0!==capi.sqlite3_strlike("%.txt", "foo.xtx", 0));
       }finally{
-        st.finalize();
+        rc = st.finalize();
       }
       T.assert(!st.pointer)
-        .assert(0===this.db.openStatementCount());
+        .assert(0===this.db.openStatementCount())
+        .assert(0===rc);
 
       T.mustThrowMatching(()=>new sqlite3.oo1.Stmt("hi"), function(err){
         return (err instanceof sqlite3.SQLite3Error)
@@ -1460,9 +1462,11 @@ self.sqlite3InitModule = sqlite3InitModule;
         T.assert(0===st.columnCount);
         const ndx = st.getParamIndex(':b');
         T.assert(1===ndx);
-        st.bindAsBlob(ndx, "ima blob").reset(true);
+        st.bindAsBlob(ndx, "ima blob")
+          /*step() skipped intentionally*/.reset(true);
       } finally {
-        st.finalize();
+        T.assert(0===st.finalize())
+          .assert(undefined===st.finalize());        
       }
 
       try {

diff --git a/manifest b/manifest
index 4c2feb650ebea4a1334f3efb0820a783c9f98bea..e8ea4b40d3bd05aa1fed1cb4820d9ba2930d90b2 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C sqlite3.oo1.Stmt.reset()\sand\sfinalize()\snow\sthrow\sif\stheir\sunderlying\sC-level\sAPIs\sreturn\snon-0,\sin\sorder\sto\savoid\ssilent\sfailure\sin\scertain\slocking-related\scases.
-D 2023-05-19T16:39:02.716
+C oo1.Stmt.finalize()\sno\slonger\sthrows,\sbut\sinstead\sreturns\sthe\ssame\sas\sthe\sC\sAPI.\soo1.DB.exec()\snow\striggers\sthe\sINSERT...RETURNING\slocking\sfailure\sas\san\sexception\svia\sreset()\sinstead\sof\sfinalize().\sSome\scode-adjacent\sinternal\sAPI\srenaming\sfor\sclarity's\ssake.
+D 2023-05-19T17:52:24.569
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -494,7 +494,7 @@ F ext/wasm/api/post-js-header.js 47b6b281f39ad59fa6e8b658308cd98ea292c286a68407b
 F ext/wasm/api/pre-js.c-pp.js ad906703f7429590f2fbf5e6498513bf727a1a4f0ebfa057afb08161d7511219
 F ext/wasm/api/sqlite3-api-cleanup.js cc21e3486da748463e02bbe51e2464c6ac136587cdfd5aa00cd0b5385f6ca808
 F ext/wasm/api/sqlite3-api-glue.js f1b2dcb944de5138bb5bd9a1559d2e76a4f3ec25260963d709e8237476688803
-F ext/wasm/api/sqlite3-api-oo1.js ac488a0e04d8b247f6cbc1b13b6ba54fac52342a80a00e1d6b17eab12f46aa59
+F ext/wasm/api/sqlite3-api-oo1.js c6a258fce7d833d48220d7e4a18512fa16f79e501e4fd371e280cdab842cd4c0
 F ext/wasm/api/sqlite3-api-prologue.js 17f4ec398ba34c5c666fea8e8c4eb82064a35b302f2f2eb355283cd8d3f68ed5
 F ext/wasm/api/sqlite3-api-worker1.js 40a5b1813fcbe789f23ae196c833432c8c83e7054d660194ddfc51eab1c5b9bf
 F ext/wasm/api/sqlite3-license-version-header.js 0c807a421f0187e778dc1078f10d2994b915123c1223fe752b60afdcd1263f89
@@ -546,7 +546,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 8f3507b22f829818a37fc44ffa81e84b4192a2ebde6a351d14b9009e136883d9
+F ext/wasm/tester1.c-pp.js 6722037cdafb25ad0415f4561bd216427b36a45ff9f51096c370e374725ba3a9
 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
@@ -2070,11 +2070,9 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P 3bb1d7b376b740c1a7bca82a8908840a9e6e9c1474c4f0fedf3d7eb917305b99
-Q +487ae12c9a21e5862bd590bbb1030c39734657d52136cf67b98c7545e6ecbe1c
-Q +d29d62cf7658aeb49f3c8a5d0b0809d945ebc9b79379a255eb88f771d2a2c430
-Q +f23eb5c6d36546ee1e181a03660e0b2dc8005bba24bee8bae594b0c78bd152cd
-R 516f611572496d6abedf96adce1c0fa8
+P db36a9ef5958cd761ca5bde697cc7cd83328dcc24d818023b813f6f860052754
+Q +4ee6b3aa531b980acea4c4b58ee256e765c5105100468928def3d4c9825fa9bc
+R 6ebf125e5dc2c49dd5a49418de89c8ad
 U stephan
-Z d47c990271222ef793409d1dd7931763
+Z 945dcd8e12e5cd02ef0bc8b720828276
 # Remove this line to create a well-formed Fossil manifest.

diff --git a/manifest.uuid b/manifest.uuid
index 6ef57a5dfe9f87a0285afbf037420b45f234d277..cf97a20913bef39cd17d77801af52d99a262bd89 100644 (file)
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-db36a9ef5958cd761ca5bde697cc7cd83328dcc24d818023b813f6f860052754
\ No newline at end of file
+15f105c78268fb68a4713da444452f5d7b038019b21024302beebb6b26ff2244
\ No newline at end of file