From 1954bc8734873b991b50adeb7e134a404752eed1 Mon Sep 17 00:00:00 2001 From: Carlo Piovesan Date: Wed, 30 Oct 2024 13:32:47 +0100 Subject: [PATCH 1/4] Fix duckdb_web_query_run_buffer memory managment in case of error --- packages/duckdb-wasm/src/bindings/bindings_base.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/duckdb-wasm/src/bindings/bindings_base.ts b/packages/duckdb-wasm/src/bindings/bindings_base.ts index 159e4bd25..ea5334e63 100644 --- a/packages/duckdb-wasm/src/bindings/bindings_base.ts +++ b/packages/duckdb-wasm/src/bindings/bindings_base.ts @@ -169,6 +169,7 @@ export abstract class DuckDBBindingsBase implements DuckDBBindings { bufferOfs.set(BUF); const [s, d, n] = callSRet(this.mod, 'duckdb_web_query_run_buffer', ['number', 'number', 'number'], [conn, bufferPtr, BUF.length]); if (s !== StatusCode.SUCCESS) { + this.mod._free(bufferPtr); throw new Error(readString(this.mod, d, n)); } const res = copyBuffer(this.mod, d, n); From e89d2d7845cac2848ed7aaef9f2540f3e5cf3045 Mon Sep 17 00:00:00 2001 From: Carlo Piovesan Date: Wed, 30 Oct 2024 13:27:52 +0100 Subject: [PATCH 2/4] Move from duckdb_web_tokenize to duckdb_web_tokenize_buffer --- lib/CMakeLists.txt | 1 + lib/src/webdb_api.cc | 7 +++++++ packages/duckdb-wasm/src/bindings/bindings_base.ts | 8 +++++++- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index b3588fbf6..949dfe141 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -333,6 +333,7 @@ if(EMSCRIPTEN) _duckdb_web_query_run_buffer, \ _duckdb_web_reset, \ _duckdb_web_tokenize, \ + _duckdb_web_tokenize_buffer, \ _duckdb_web_udf_scalar_create \ ]' \ -s EXPORTED_RUNTIME_METHODS='[\"ccall\", \"stackSave\", \"stackAlloc\", \"stackRestore\"]' \ diff --git a/lib/src/webdb_api.cc b/lib/src/webdb_api.cc index 33448a518..4b155784b 100644 --- a/lib/src/webdb_api.cc +++ b/lib/src/webdb_api.cc @@ -151,6 +151,13 @@ void duckdb_web_tokenize(WASMResponse* packed, const char* query) { auto tokens = webdb.Tokenize(query); WASMResponseBuffer::Get().Store(*packed, arrow::Result(std::move(tokens))); } +/// Tokenize a query +void duckdb_web_tokenize_buffer(WASMResponse* packed, const uint8_t* buffer, size_t buffer_length) { + GET_WEBDB(*packed); + std::string_view query(reinterpret_cast(buffer), buffer_length); + auto tokens = webdb.Tokenize(query); + WASMResponseBuffer::Get().Store(*packed, arrow::Result(std::move(tokens))); +} /// Create scalar UDF queries void duckdb_web_udf_scalar_create(WASMResponse* packed, ConnectionHdl connHdl, const char* args) { auto c = reinterpret_cast(connHdl); diff --git a/packages/duckdb-wasm/src/bindings/bindings_base.ts b/packages/duckdb-wasm/src/bindings/bindings_base.ts index ea5334e63..001dba4ae 100644 --- a/packages/duckdb-wasm/src/bindings/bindings_base.ts +++ b/packages/duckdb-wasm/src/bindings/bindings_base.ts @@ -134,11 +134,17 @@ export abstract class DuckDBBindingsBase implements DuckDBBindings { /** Tokenize a script */ public tokenize(text: string): ScriptTokens { - const [s, d, n] = callSRet(this.mod, 'duckdb_web_tokenize', ['string'], [text]); + const BUF = TEXT_ENCODER.encode(text); + const bufferPtr = this.mod._malloc(BUF.length ); + const bufferOfs = this.mod.HEAPU8.subarray(bufferPtr, bufferPtr + BUF.length ); + bufferOfs.set(BUF); + const [s, d, n] = callSRet(this.mod, 'duckdb_web_tokenize_buffer', ['number', 'number'], [bufferPtr, BUF.length]); if (s !== StatusCode.SUCCESS) { + this.mod._free(bufferPtr); throw new Error(readString(this.mod, d, n)); } const res = readString(this.mod, d, n); + this.mod._free(bufferPtr); dropResponseBuffers(this.mod); return JSON.parse(res) as ScriptTokens; } From feb1aba2232ad76f4f20af5d5a496451b3442684 Mon Sep 17 00:00:00 2001 From: Carlo Piovesan Date: Wed, 30 Oct 2024 13:41:50 +0100 Subject: [PATCH 3/4] Move from duckdb_web_pending_query_start to duckdb_web_pending_query_start_buffer --- lib/CMakeLists.txt | 1 + lib/src/webdb_api.cc | 8 ++++++++ packages/duckdb-wasm/src/bindings/bindings_base.ts | 14 ++++++++------ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index 949dfe141..de246193d 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -324,6 +324,7 @@ if(EMSCRIPTEN) _duckdb_web_pending_query_cancel, \ _duckdb_web_pending_query_poll, \ _duckdb_web_pending_query_start, \ + _duckdb_web_pending_query_start_buffer, \ _duckdb_web_prepared_close, \ _duckdb_web_prepared_create, \ _duckdb_web_prepared_run, \ diff --git a/lib/src/webdb_api.cc b/lib/src/webdb_api.cc index 4b155784b..0c8593e6d 100644 --- a/lib/src/webdb_api.cc +++ b/lib/src/webdb_api.cc @@ -210,6 +210,14 @@ void duckdb_web_pending_query_start(WASMResponse* packed, ConnectionHdl connHdl, auto r = c->PendingQuery(script, allow_stream_result); WASMResponseBuffer::Get().Store(*packed, std::move(r)); } +/// Start a pending query +void duckdb_web_pending_query_start_buffer(WASMResponse* packed, ConnectionHdl connHdl, const uint8_t* buffer, + size_t buffer_length, bool allow_stream_result) { + auto c = reinterpret_cast(connHdl); + std::string_view S(reinterpret_cast(buffer), buffer_length); + auto r = c->PendingQuery(S, allow_stream_result); + WASMResponseBuffer::Get().Store(*packed, std::move(r)); +} /// Poll a pending query void duckdb_web_pending_query_poll(WASMResponse* packed, ConnectionHdl connHdl, const char* script) { auto c = reinterpret_cast(connHdl); diff --git a/packages/duckdb-wasm/src/bindings/bindings_base.ts b/packages/duckdb-wasm/src/bindings/bindings_base.ts index 001dba4ae..4d836de98 100644 --- a/packages/duckdb-wasm/src/bindings/bindings_base.ts +++ b/packages/duckdb-wasm/src/bindings/bindings_base.ts @@ -190,19 +190,21 @@ export abstract class DuckDBBindingsBase implements DuckDBBindings { * Results can then be fetched using `fetchQueryResults` */ public startPendingQuery(conn: number, text: string, allowStreamResult: boolean = false): Uint8Array | null { - const [s, d, n] = callSRet( - this.mod, - 'duckdb_web_pending_query_start', - ['number', 'string', 'boolean'], - [conn, text, allowStreamResult], - ); + const BUF = TEXT_ENCODER.encode(text); + const bufferPtr = this.mod._malloc(BUF.length ); + const bufferOfs = this.mod.HEAPU8.subarray(bufferPtr, bufferPtr + BUF.length ); + bufferOfs.set(BUF); + const [s, d, n] = callSRet(this.mod, 'duckdb_web_pending_query_start_buffer', ['number', 'number', 'number', 'boolean'], [conn, bufferPtr, BUF.length, allowStreamResult]); if (s !== StatusCode.SUCCESS) { + this.mod._free(bufferPtr); throw new Error(readString(this.mod, d, n)); } if (d == 0) { + this.mod._free(bufferPtr); return null; } const res = copyBuffer(this.mod, d, n); + this.mod._free(bufferPtr); dropResponseBuffers(this.mod); return res; } From a3917e77cfac121b2e7286e6f0bf7648ec8f8995 Mon Sep 17 00:00:00 2001 From: Carlo Piovesan Date: Wed, 30 Oct 2024 14:54:59 +0100 Subject: [PATCH 4/4] Actually free just after the call --- packages/duckdb-wasm/src/bindings/bindings_base.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/duckdb-wasm/src/bindings/bindings_base.ts b/packages/duckdb-wasm/src/bindings/bindings_base.ts index 4d836de98..a1d047207 100644 --- a/packages/duckdb-wasm/src/bindings/bindings_base.ts +++ b/packages/duckdb-wasm/src/bindings/bindings_base.ts @@ -139,12 +139,11 @@ export abstract class DuckDBBindingsBase implements DuckDBBindings { const bufferOfs = this.mod.HEAPU8.subarray(bufferPtr, bufferPtr + BUF.length ); bufferOfs.set(BUF); const [s, d, n] = callSRet(this.mod, 'duckdb_web_tokenize_buffer', ['number', 'number'], [bufferPtr, BUF.length]); + this.mod._free(bufferPtr); if (s !== StatusCode.SUCCESS) { - this.mod._free(bufferPtr); throw new Error(readString(this.mod, d, n)); } const res = readString(this.mod, d, n); - this.mod._free(bufferPtr); dropResponseBuffers(this.mod); return JSON.parse(res) as ScriptTokens; } @@ -174,13 +173,12 @@ export abstract class DuckDBBindingsBase implements DuckDBBindings { const bufferOfs = this.mod.HEAPU8.subarray(bufferPtr, bufferPtr + BUF.length); bufferOfs.set(BUF); const [s, d, n] = callSRet(this.mod, 'duckdb_web_query_run_buffer', ['number', 'number', 'number'], [conn, bufferPtr, BUF.length]); + this.mod._free(bufferPtr); if (s !== StatusCode.SUCCESS) { - this.mod._free(bufferPtr); throw new Error(readString(this.mod, d, n)); } const res = copyBuffer(this.mod, d, n); dropResponseBuffers(this.mod); - this.mod._free(bufferPtr); return res; } /** @@ -195,16 +193,14 @@ export abstract class DuckDBBindingsBase implements DuckDBBindings { const bufferOfs = this.mod.HEAPU8.subarray(bufferPtr, bufferPtr + BUF.length ); bufferOfs.set(BUF); const [s, d, n] = callSRet(this.mod, 'duckdb_web_pending_query_start_buffer', ['number', 'number', 'number', 'boolean'], [conn, bufferPtr, BUF.length, allowStreamResult]); + this.mod._free(bufferPtr); if (s !== StatusCode.SUCCESS) { - this.mod._free(bufferPtr); throw new Error(readString(this.mod, d, n)); } if (d == 0) { - this.mod._free(bufferPtr); return null; } const res = copyBuffer(this.mod, d, n); - this.mod._free(bufferPtr); dropResponseBuffers(this.mod); return res; }