diff --git a/ext/wasm/api/sqlite3-api-oo1.js b/ext/wasm/api/sqlite3-api-oo1.js index d46ee8dd43..4da629a6bc 100644 --- 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 e16b0f95c7..fc86b7f657 100644 --- 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 dc00b05515..1fb1328874 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Improved\sdocumentation\sfor\ssqlite3_reset(),\sin\sresponse\sto\n[forum:/forumpost/a72bab3dea|forum\spost\sa72bab3dea]. -D 2023-05-19T16:42:49.388 +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:50:16.695 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,8 +2070,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 487ae12c9a21e5862bd590bbb1030c39734657d52136cf67b98c7545e6ecbe1c -R 39b64ba449418cb0ab801f0a9bec4df8 -U drh -Z 46fed3a751762448fe0476ef3a96f8d1 +P 1dfeb3dceee8f30daf5462683f264b9de23e7068e036e70b11ee1b608ac2f7fa +R a12ad3984ab6086345836bc1a346ae92 +U stephan +Z a9829d02233e35008022e5ee0b47f59f # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index c6bcd339eb..41d50dbeb6 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -1dfeb3dceee8f30daf5462683f264b9de23e7068e036e70b11ee1b608ac2f7fa \ No newline at end of file +4ee6b3aa531b980acea4c4b58ee256e765c5105100468928def3d4c9825fa9bc \ No newline at end of file