From: stephan Date: Fri, 19 May 2023 16:39:02 +0000 (+0000) Subject: sqlite3.oo1.Stmt.reset() and finalize() now throw if their underlying C-level APIs... X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=1c1dfc1d468a8d2dd82d0392e16a77f22fa71e6f;p=thirdparty%2Fsqlite.git sqlite3.oo1.Stmt.reset() and finalize() now throw if their underlying C-level APIs return non-0, in order to avoid silent failure in certain locking-related cases. FossilOrigin-Name: db36a9ef5958cd761ca5bde697cc7cd83328dcc24d818023b813f6f860052754 --- diff --git a/ext/wasm/api/sqlite3-api-oo1.js b/ext/wasm/api/sqlite3-api-oo1.js index 1f244a7c5b..d46ee8dd43 100644 --- a/ext/wasm/api/sqlite3-api-oo1.js +++ b/ext/wasm/api/sqlite3-api-oo1.js @@ -55,6 +55,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ if(sqliteResultCode){ if(dbPtr instanceof DB) dbPtr = dbPtr.pointer; toss3( + sqliteResultCode, "sqlite3 result code",sqliteResultCode+":", (dbPtr ? capi.sqlite3_errmsg(dbPtr) @@ -503,6 +504,9 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ "Not an error." The various non-0 non-error codes need to be checked for in client code where they are expected. + The thrown exception's `resultCode` property will be the value of + the second argument to this function. + If it does not throw, it returns its first argument. */ DB.checkRc = (db,resultCode)=>checkSqlite3Rc(db,resultCode); @@ -779,7 +783,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ should return: A) The default value is (usually) `"this"`, meaning that the - DB object itself should be returned. The exceptions is if + DB object itself should be returned. The exception is if the caller passes neither of `callback` nor `returnValue` but does pass an explicit `rowMode` then the default `returnValue` is `"resultRows"`, described below. @@ -899,11 +903,11 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ sqlite3.config.warn("DB.exec() is propagating exception",opt,e); throw e; }*/finally{ + wasm.scopedAllocPop(stack); if(stmt){ delete stmt._isLocked; stmt.finalize(); } - wasm.scopedAllocPop(stack); } return arg.returnVal(); }/*exec()*/, @@ -1256,7 +1260,7 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ not throw, it returns this object. */ checkRc: function(resultCode){ - return DB.checkRc(this, resultCode); + return checkSqlite3Rc(this, resultCode); } }/*DB.prototype*/; @@ -1420,24 +1424,40 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ Stmt.prototype = { /** "Finalizes" this statement. This is a no-op if the - statement has already been finalizes. Returns + statement has already been finalized. Returns undefined. 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. */ finalize: function(){ if(this.pointer){ affirmUnlocked(this,'finalize()'); - delete __stmtMap.get(this.db)[this.pointer]; - capi.sqlite3_finalize(this.pointer); + const rc = capi.sqlite3_finalize(this.pointer), + db = this.db; + delete __stmtMap.get(db)[this.pointer]; __ptrMap.delete(this); delete this._mayGet; delete this.parameterCount; - delete this.db; delete this._isLocked; + delete this.db; + checkSqlite3Rc(db, rc); } }, - /** Clears all bound values. Returns this object. - Throws if this statement has been finalized. */ + /** + Clears all bound values. Returns this object. Throws if this + statement has been finalized or if modification of the + statement is currently illegal (e.g. in the per-row callback of + a DB.exec() call). + */ clearBindings: function(){ affirmUnlocked(affirmStmtOpen(this), 'clearBindings()') capi.sqlite3_clear_bindings(this.pointer); @@ -1445,19 +1465,25 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ return this; }, /** - Resets this statement so that it may be step()ed again - from the beginning. Returns this object. Throws if this - statement has been finalized. + 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. 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. */ reset: function(alsoClearBinds){ affirmUnlocked(this,'reset()'); if(alsoClearBinds) this.clearBindings(); - capi.sqlite3_reset(affirmStmtOpen(this).pointer); + const rc = capi.sqlite3_reset(affirmStmtOpen(this).pointer); this._mayGet = false; + checkSqlite3Rc(this.db, rc); return this; }, /** @@ -1641,11 +1667,9 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ return this.reset(); }, /** - Functions like step() except that it finalizes this statement - immediately after stepping unless the step cannot be performed - because the statement is locked. Throws on error, but any error - other than the statement-is-locked case will also trigger - finalization of this statement. + Functions like step() except that it calls finalize() on this + statement immediately after stepping, even if the step() call + throws. On success, it returns true if the step indicated that a row of data was available, else it returns false. @@ -1657,8 +1681,19 @@ globalThis.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ ``` */ stepFinalize: function(){ - const rc = this.step(); - this.finalize(); + let rc, err; + try{ + rc = this.step(); + }catch(e){ + err = e; + } + if(err){ + try{this.finalize()} + catch(x){/*ignored*/} + throw err; + }else{ + this.finalize(); + } return rc; }, /** diff --git a/ext/wasm/tester1-worker.html b/ext/wasm/tester1-worker.html index 03e1f02b02..e768c3d6c3 100644 --- a/ext/wasm/tester1-worker.html +++ b/ext/wasm/tester1-worker.html @@ -45,7 +45,7 @@ if(urlParams.has('esm')){ logHtml('warning',"Attempting to run an ES6 Worker Module, "+ "which is not supported by all browsers! "+ - "e.g. Firefox (as of 2022-12) cannot do this."); + "e.g. Firefox (as of 2023-05) cannot do this."); workerArgs.push("tester1.mjs",{type:"module"}); document.querySelectorAll('title,#color-target').forEach((e)=>{ e.innerText = "sqlite3 tester #1: ES6 Worker Module"; diff --git a/ext/wasm/tester1.c-pp.js b/ext/wasm/tester1.c-pp.js index 1e1b91e65c..e16b0f95c7 100644 --- a/ext/wasm/tester1.c-pp.js +++ b/ext/wasm/tester1.c-pp.js @@ -1166,7 +1166,8 @@ self.sqlite3InitModule = sqlite3InitModule; try{db.checkRc(rc)} catch(e){ex = e} T.assert(ex instanceof sqlite3.SQLite3Error) - .assert(0===ex.message.indexOf("sqlite3 result code")) + .assert(capi.SQLITE_MISUSE===ex.resultCode) + .assert(0===ex.message.indexOf("SQLITE_MISUSE: sqlite3 result code")) .assert(ex.message.indexOf("Invalid SQL")>0); T.assert(db === db.checkRc(0)) .assert(db === sqlite3.oo1.DB.checkRc(db,0)) @@ -1334,8 +1335,8 @@ self.sqlite3InitModule = sqlite3InitModule; sql:['CREATE TABLE t(a,b);', // ^^^ using TEMP TABLE breaks the db export test "INSERT INTO t(a,b) VALUES(1,2),(3,4),", - "(?,?),('blob',X'6869')"/*intentionally missing semicolon to test for - off-by-one bug in string-to-WASM conversion*/], + "(?,?)"/*intentionally missing semicolon to test for + off-by-one bug in string-to-WASM conversion*/], saveSql: list, bind: [5,6] }); @@ -1343,12 +1344,20 @@ self.sqlite3InitModule = sqlite3InitModule; T.assert(rc === db) .assert(2 === list.length) .assert('string'===typeof list[1]) - .assert(4===db.changes()) + .assert(3===db.changes()) .assert(this.progressHandlerCount > 0, "Expecting progress callback.") if(wasm.bigIntEnabled){ - T.assert(4n===db.changes(false,true)); + T.assert(3n===db.changes(false,true)); } + rc = db.exec({ + sql: "INSERT INTO t values('blob',X'6869') RETURNING 13", + rowMode: 0 + }); + T.assert(Array.isArray(rc)) + .assert(1===rc.length) + .assert(13 === rc[0]) + .assert(1===db.changes()); let vals = db.selectValues('select a from t order by a limit 2'); T.assert( 2 === vals.length ) diff --git a/manifest b/manifest index f92c89068b..4c2feb650e 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Correct\stypo\sin\san\s'extern'\sdecl\sname,\sreported\sin\s[forum:1d4342156439233b|forum\spost\s1d4342156439233b]. -D 2023-05-19T12:46:38.970 +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 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 410977a7a24db8fb8824307fbacf18d79a36de5838e827cf2ed70b938d6bc50e +F ext/wasm/api/sqlite3-api-oo1.js ac488a0e04d8b247f6cbc1b13b6ba54fac52342a80a00e1d6b17eab12f46aa59 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 @@ -544,9 +544,9 @@ F ext/wasm/sql/000-mandelbrot.sql 775337a4b80938ac8146aedf88808282f04d02d983d826 F ext/wasm/sql/001-sudoku.sql 35b7cb7239ba5d5f193bc05ec379bcf66891bce6f2a5b3879f2f78d0917299b5 F ext/wasm/test-opfs-vfs.html 1f2d672f3f3fce810dfd48a8d56914aba22e45c6834e262555e685bce3da8c3f F ext/wasm/test-opfs-vfs.js f09266873e1a34d9bdb6d3981ec8c9e382f31f215c9fd2f9016d2394b8ae9b7b -F ext/wasm/tester1-worker.html 258d08f1ba9cc2d455958751e26be833893cf9ff7853e9436e593e1f778a386b +F ext/wasm/tester1-worker.html ebc4b820a128963afce328ecf63ab200bd923309eb939f4110510ab449e9814c F ext/wasm/tester1.c-pp.html 1c1bc78b858af2019e663b1a31e76657b73dc24bede28ca92fbe917c3a972af2 -F ext/wasm/tester1.c-pp.js 2682abac377af9625f23837b7cfdbd0d8e0d5fee4d2acf3c98e1564785ccc479 +F ext/wasm/tester1.c-pp.js 8f3507b22f829818a37fc44ffa81e84b4192a2ebde6a351d14b9009e136883d9 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,9 +2070,11 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P 0a0b7a2d3178f1aa650acd1d729566c889e27c9714463437a7f17c08e992ffd3 -Q +6ac18827d8c5be35442dc452af703023c759916c179f165c4e54b3e430a8f7b0 -R ce33448fbda4a581fbcc24c472d3083f +P 3bb1d7b376b740c1a7bca82a8908840a9e6e9c1474c4f0fedf3d7eb917305b99 +Q +487ae12c9a21e5862bd590bbb1030c39734657d52136cf67b98c7545e6ecbe1c +Q +d29d62cf7658aeb49f3c8a5d0b0809d945ebc9b79379a255eb88f771d2a2c430 +Q +f23eb5c6d36546ee1e181a03660e0b2dc8005bba24bee8bae594b0c78bd152cd +R 516f611572496d6abedf96adce1c0fa8 U stephan -Z 176f2ce1aa7a9447f09902f760d983a4 +Z d47c990271222ef793409d1dd7931763 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 35ce6d7052..6ef57a5dfe 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -3bb1d7b376b740c1a7bca82a8908840a9e6e9c1474c4f0fedf3d7eb917305b99 \ No newline at end of file +db36a9ef5958cd761ca5bde697cc7cd83328dcc24d818023b813f6f860052754 \ No newline at end of file