From 7197f0a8f22e66e9535684655dde72e33b94eb12 Mon Sep 17 00:00:00 2001 From: stephan Date: Mon, 26 Dec 2022 13:00:58 +0000 Subject: [PATCH] Add JS infrastructure which enables sqlite3.capi.sqlite3_close_v2() to clean up stale JS-to-WASM collation function conversions installed on behalf of a given db handle. The same for UDF mappings is TODO. FossilOrigin-Name: 0e69b2c379e61893c7db8a9c9d270650f2bd63b6cea30811d41136392a2e4f04 --- ext/wasm/api/sqlite3-api-glue.js | 120 ++++++++++++++++++++++--------- manifest | 12 ++-- manifest.uuid | 2 +- 3 files changed, 94 insertions(+), 40 deletions(-) diff --git a/ext/wasm/api/sqlite3-api-glue.js b/ext/wasm/api/sqlite3-api-glue.js index 94554e0b55..7db149ba5a 100644 --- a/ext/wasm/api/sqlite3-api-glue.js +++ b/ext/wasm/api/sqlite3-api-glue.js @@ -731,43 +731,95 @@ self.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ ); }; + /** + __dbCleanupMap is infrastructure for recording registration of + UDFs and collations so that sqlite3_close_v2() can clean up any + automated JS-to-WASM function conversions installed by those. + */ + const __argPDb = (pDb)=>wasm.xWrap.argAdapter('sqlite3*')(pDb); + const __argStr = (str)=>wasm.isPtr(str) ? wasm.cstrToJs(str) : str; + const __dbCleanupMap = function( + pDb, mode/*0=remove, >0=create if needed, <0=do not create if missing*/ + ){ + pDb = __argPDb(pDb); + let m = this.dbMap.get(pDb); + if(!mode){ + this.dbMap.delete(pDb); + return m; + }else if(!m && mode>0){ + this.dbMap.set(pDb, (m = Object.create(null))); + } + return m; + }.bind(Object.assign(Object.create(null),{ + dbMap: new Map + })); + + __dbCleanupMap.addCollation = function(pDb, name){ + const m = __dbCleanupMap(pDb, 1); + if(!m.collation) m.collation = new Set; + m.collation.add(__argStr(name).toLowerCase()); + }; + + /** + Intended to be called _only_ from sqlite3_close_v2(), + passed its non-0 db argument. + + This function freees up certain automatically-installed WASM + function bindings which were installed on behalf of the given db, + as those may otherwise leak. + + Notable caveat: this is only ever run via + sqlite3.capi.sqlite3_close_v2(). If a client, for whatever + reason, uses sqlite3.wasm.exports.sqlite3_close_v2() (the + function directly exported from WASM), this cleanup will not + happen. + + This is not a silver bullet for avoiding automation-related + leaks but represents "an honest effort." + + The issue being addressed here is covered at: + + https://sqlite.org/wasm/doc/trunk/api-c-style.md#convert-func-ptr + */ + __dbCleanupMap.cleanup = function(pDb){ + pDb = __argPDb(pDb); + //wasm.xWrap.FuncPtrAdapter.debugFuncInstall = true; + /** + 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. + */ + 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*/} + const m = __dbCleanupMap(pDb, 0); + if(!m) return; + if(m.collation){ + for(const name of m.collation){ + try{ + capi.sqlite3_create_collation_v2( + pDb, name, capi.SQLITE_UTF8, 0, 0, 0 + ); + }catch(e){ + /*ignored*/ + } + } + delete m.collation; + } + if(m.udf){ + //TODO: map and clean up UDFs. + } + }; + {/* 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*/} + try{__dbCleanupMap.cleanup(pDb)} catch(e){/*ignored*/} } return __sqlite3CloseV2(pDb); }; @@ -844,7 +896,9 @@ self.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ return __errEncoding(pDb); } try{ - return __sqlite3CreateCollationV2(pDb, zName, eTextRep, pArg, xCompare, xDestroy); + const rc = __sqlite3CreateCollationV2(pDb, zName, eTextRep, pArg, xCompare, xDestroy); + if(xCompare) __dbCleanupMap.addCollation(pDb, zName); + return rc; }catch(e){ return util.sqlite3_wasm_db_error(pDb, e); } diff --git a/manifest b/manifest index c0dd642d58..e61313a295 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Reimplement\ssqlite3.capi.sqlite3_close_v2()\sand\ssqlite3session_delete()\sas\sa\shand-written\sbindings\sso\sthat\sthey\scan\sattempt\sto\sclean\sup\scertain\s(potentially)\sFuncPtrAdapter-installed\sfunctions\sbefore\sclosing.\sCorrect\sthe\screate-function\sfamily\sof\sJS-to-function-pointer\sautomated\sconversions\sto\sinclude\sthe\sUDF's\sarity\sas\spart\sof\sthe\smapping's\skey\sso\sthat\s(un)binding\sa\sUDF\sto\sdifferent\sfunctions\sfor\sdifferent\sarities\sworks\s(and\sadd\stests\sconfirming\sit).\sCorrect\sa\sbroken\sdoc\slink\sin\smodule-symbols.html. -D 2022-12-26T11:13:09.162 +C Add\sJS\sinfrastructure\swhich\senables\ssqlite3.capi.sqlite3_close_v2()\sto\sclean\sup\sstale\sJS-to-WASM\scollation\sfunction\sconversions\sinstalled\son\sbehalf\sof\sa\sgiven\sdb\shandle.\sThe\ssame\sfor\sUDF\smappings\sis\sTODO. +D 2022-12-26T13:00:58.115 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -503,7 +503,7 @@ F ext/wasm/api/post-js-footer.js cd0a8ec768501d9bd45d325ab0442037fb0e33d1f3b4f08 F ext/wasm/api/post-js-header.js 47b6b281f39ad59fa6e8b658308cd98ea292c286a68407b35ff3ed9cfd281a62 F ext/wasm/api/pre-js.c-pp.js b88499dc303c21fc3f55f2c364a0f814f587b60a95784303881169f9e91c1d5f F ext/wasm/api/sqlite3-api-cleanup.js 680d5ccfff54459db136a49b2199d9f879c8405d9c99af1dda0cc5e7c29056f4 -F ext/wasm/api/sqlite3-api-glue.js 3bfe06cf019880a14353fe16102d8515e2cfd5b6d01941e54e2145d7298e0bb1 +F ext/wasm/api/sqlite3-api-glue.js 8e6336cd5c6e404b1460a196196eb6362b13de4ec24544ef1a3a1a4132245d9c F ext/wasm/api/sqlite3-api-oo1.js 959be9a922d1f012b4a25e7b763c112220bb0efb989f56b82a776ab1ccebe72d F ext/wasm/api/sqlite3-api-prologue.js 3792a703ea15be8d4393a99992862c285d62732d760cec95226dc5ec2781d920 F ext/wasm/api/sqlite3-api-worker1.js c9ef8865f072e61251260b218aa4ed614a21a25e9e3cc6f22acf81794d32fc0b @@ -2067,8 +2067,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 b6dc80cbf63ed521ef8f878fba24b0110d61813763ca7bfbcfb0a145656b300a -R 06748c7887c5b78d22b2056b9803d8a1 +P 60b262ef0f57b162c2566b12e70685a92afb00b441332ea7a6540fcb188cc7af +R ad57ccb8cecad2b9daaaef2f36f544d6 U stephan -Z d4141c6258473aed1cc5843079c53a98 +Z 7341e7115e45a0f5c9a909c3a65ff344 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index e9f0cb9351..23b40688f4 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -60b262ef0f57b162c2566b12e70685a92afb00b441332ea7a6540fcb188cc7af \ No newline at end of file +0e69b2c379e61893c7db8a9c9d270650f2bd63b6cea30811d41136392a2e4f04 \ No newline at end of file