From d99722da064a1734d0f5df5661a2c275aba06e88 Mon Sep 17 00:00:00 2001 From: DjDeveloper <43033058+DjDeveloperr@users.noreply.github.com> Date: Fri, 24 Dec 2021 08:50:52 +0530 Subject: [PATCH] fix segmentation fault (#5) --- .gitignore | 3 ++ bench/backend.ts | 1 + bench/main.ts | 46 ++++++++++++++++++++++++-- bench/native.ts | 1 + bench/wasm.ts | 1 + src/database.ts | 75 ++++++++++++++++++++++++++++++----------- src/ffi.ts | 86 ++++++++++++++++++++---------------------------- src/util.ts | 24 -------------- test/test.ts | 8 ++++- 9 files changed, 148 insertions(+), 97 deletions(-) diff --git a/.gitignore b/.gitignore index 06de9cc..dc1eab2 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,6 @@ sqlite_test/ *.pdb *.dll *.so +async-sqlite3 +*.db-shm +*.db-wal diff --git a/bench/backend.ts b/bench/backend.ts index 8656161..fa02b81 100644 --- a/bench/backend.ts +++ b/bench/backend.ts @@ -5,6 +5,7 @@ export interface Prepare { export interface Backend { name: string; + query(sql: string): unknown[]; execute(sql: string, params: unknown[]): void; prepare(sql: string): Prepare; close(): void; diff --git a/bench/main.ts b/bench/main.ts index 4217839..1a50e64 100644 --- a/bench/main.ts +++ b/bench/main.ts @@ -2,17 +2,20 @@ import native from "./native.ts"; import wasm from "./wasm.ts"; import { Backend } from "./backend.ts"; +let level = 0; + function log(type: string, msg: string) { - console.log(`%c${type} %c${msg}`, "color: #0DBC79", ""); + console.log(`${" ".repeat(level)}%c${type} %c${msg}`, "color: #0DBC79", ""); } -const ROWS = 100; +const ROWS = 10; const ITERS = 10; const backends: Backend[] = [native, wasm]; for (const backend of backends) { log("Bench", backend.name); + level++; backend.execute("pragma journal_mode = WAL", []); backend.execute("pragma synchronous = normal", []); @@ -23,6 +26,9 @@ for (const backend of backends) { [], ); + log("Insert", "Bench start ->"); + level++; + let total = 0; let min!: number, max!: number; for (let iter = 0; iter < ITERS; iter++) { @@ -43,7 +49,6 @@ for (const backend of backends) { total += took; } - backend.close(); log( "Result", @@ -53,4 +58,39 @@ for (const backend of backends) { Math.abs(max - min).toFixed(2) }ms`, ); + level--; + + log("Query", "Bench start ->"); + level++; + + total = 0, min = undefined as any, max = undefined as any; + for (let iter = 0; iter < ITERS; iter++) { + const now = performance.now(); + backend.query("select * from test"); + const took = performance.now() - now; + + if (min === undefined || max === undefined) { + min = max = took; + } else { + min = Math.min(min, took); + max = Math.max(max, took); + } + + total += took; + } + + log( + "Result", + `${ITERS} iter ${ROWS} rows took ${total.toFixed(2)}ms ${ + (ITERS / (total / 1000)).toFixed(2) + } iter/sec min ${min.toFixed(2)}ms max ${max.toFixed(2)}ms ±${ + Math.abs(max - min).toFixed(2) + }ms`, + ); + + level--; + + level--; + + backend.close(); } diff --git a/bench/native.ts b/bench/native.ts index e3d74dc..f5fa178 100644 --- a/bench/native.ts +++ b/bench/native.ts @@ -17,5 +17,6 @@ export default { finalize: () => prep.finalize(), }; }, + query: (sql) => db.queryArray(sql), close: () => db.close(), }; diff --git a/bench/wasm.ts b/bench/wasm.ts index 4c120a6..48e1901 100644 --- a/bench/wasm.ts +++ b/bench/wasm.ts @@ -16,5 +16,6 @@ export default { finalize: () => prep.finalize(), }; }, + query: (sql) => db.query(sql), close: () => db.close(), }; diff --git a/src/database.ts b/src/database.ts index 3826967..d40cf8a 100644 --- a/src/database.ts +++ b/src/database.ts @@ -16,7 +16,6 @@ import { sqlite3_bind_double, sqlite3_bind_int, sqlite3_bind_int64, - sqlite3_bind_null, sqlite3_bind_parameter_count, sqlite3_bind_parameter_index, sqlite3_bind_parameter_name, @@ -76,7 +75,7 @@ export class Database { #path: string; #handle: sqlite3; - /** Path of the */ + /** Path of the database file. */ get path(): string { return this.#path; } @@ -266,7 +265,7 @@ export class Row { /** Returns the names of the columns in the row. */ get columns(): string[] { - const columnCount = this.columnCount; + const columnCount = this.#stmt.columnCount; const cols = new Array(columnCount); for (let i = 0; i < columnCount; i++) { cols[i] = this.#stmt.columnName(i); @@ -276,7 +275,7 @@ export class Row { /** Returns the row as array containing columns' values. */ asArray() { - const columnCount = this.columnCount; + const columnCount = this.#stmt.columnCount; const array = new Array(columnCount); for (let i = 0; i < columnCount; i++) { array[i] = this.#stmt.column(i); @@ -286,7 +285,7 @@ export class Row { /** Returns the row as object with column names mapping to values. */ asObject = Record>(): T { - const columnCount = this.columnCount; + const columnCount = this.#stmt.columnCount; const obj: Record = {}; for (let i = 0; i < columnCount; i++) { const name = this.#stmt.columnName(i); @@ -339,6 +338,15 @@ export class PreparedStatement { return sqlite3_bind_parameter_index(this.#handle, name); } + #cstrCache = new Map(); + + #cstr(str: string) { + if (this.#cstrCache.has(str)) return this.#cstrCache.get(str)!; + const val = cstr(str); + this.#cstrCache.set(str, val); + return val; + } + /** * We need to store references to any type that involves passing pointers * to avoid V8's GC deallocating them before the statement is finalized. @@ -386,7 +394,9 @@ export class PreparedStatement { case "object": if (value === null) { - sqlite3_bind_null(this.db.unsafeRawHandle, this.#handle, index); + // By default, SQLite sets non-binded values to null. + // so this call is not needed. + // sqlite3_bind_null(this.db.unsafeRawHandle, this.#handle, index); } else if (value instanceof Uint8Array) { this.#bufferRefs.add(value); sqlite3_bind_blob( @@ -412,7 +422,7 @@ export class PreparedStatement { break; case "string": { - const buffer = cstr(value); + const buffer = this.#cstr(value); this.#bufferRefs.add(buffer); sqlite3_bind_text( this.db.unsafeRawHandle, @@ -454,19 +464,32 @@ export class PreparedStatement { } } + #cachedColCount?: number; + /** Column count in current row. */ get columnCount() { - return sqlite3_column_count(this.#handle); + if (this.#cachedColCount !== undefined) return this.#cachedColCount; + return (this.#cachedColCount = sqlite3_column_count(this.#handle)); } + #colTypeCache = new Map(); + /** Return the data type of the column at given index in current row. */ columnType(index: number): SqliteType { - return sqlite3_column_type(this.#handle, index); + if (this.#colTypeCache.has(index)) return this.#colTypeCache.get(index)!; + const type = sqlite3_column_type(this.#handle, index); + this.#colTypeCache.set(index, type); + return type; } + #colNameCache = new Map(); + /** Return the name of the column at given index in current row. */ columnName(index: number) { - return sqlite3_column_name(this.#handle, index); + if (this.#colNameCache.has(index)) return this.#colNameCache.get(index)!; + const name = sqlite3_column_name(this.#handle, index); + this.#colNameCache.set(index, name); + return name; } /** Return value of a column at given index in current row. */ @@ -486,6 +509,7 @@ export class PreparedStatement { case SqliteType.BLOB: { const blob = sqlite3_column_blob(this.#handle, index); + if (blob.value === 0n) return null; const length = sqlite3_column_bytes(this.#handle, index); const data = new Uint8Array(length); new Deno.UnsafePointerView(blob).copyInto(data); @@ -493,7 +517,7 @@ export class PreparedStatement { } default: - throw new Error(`Unsupported column type: ${this.columnType(index)}`); + return null; } } @@ -514,22 +538,35 @@ export class PreparedStatement { sqlite3_reset(this.#db.unsafeRawHandle, this.#handle); } - /** Finalize and run the prepared statement. */ + /** Adds another step to prepared statement to be executed. Don't forget to call `finalize`. */ + execute(...args: unknown[]) { + this.bindAll(...args); + this.step(); + this.reset(); + } + + /** + * Finalize and run the prepared statement. + * + * This also frees up any resources related to the statement. + * And clears all references to the buffers as they're no longer + * needed, allowing V8 to GC them. + */ finalize() { try { sqlite3_finalize(this.#db.unsafeRawHandle, this.#handle); } finally { this.#bufferRefs.clear(); + this.#colTypeCache.clear(); + this.#colNameCache.clear(); + this.#cstrCache.clear(); + this.#cachedColCount = undefined; } } - /** Adds another step to prepared statement to be executed. Don't forget to call `finalize`. */ - execute(...args: unknown[]) { - this.bindAll(...args); - this.step(); - this.reset(); - } - + /** + * Returns an iterator for rows. + */ *[Symbol.iterator]() { let row; while ((row = this.step())) { diff --git a/src/ffi.ts b/src/ffi.ts index 3217921..cf66c2f 100644 --- a/src/ffi.ts +++ b/src/ffi.ts @@ -142,7 +142,10 @@ const symbols = > { }, sqlite3_bind_null: { - parameters: ["pointer", /* sqlite3_stmt *pStmt */ "i32" /* int i */], + parameters: [ + "pointer", /* sqlite3_stmt *pStmt */ + "i32", /* int i */ + ], result: "i32", }, @@ -204,17 +207,26 @@ const symbols = > { }, sqlite3_column_double: { - parameters: ["pointer", /* sqlite3_stmt *pStmt */ "i32" /* int iCol */], + parameters: [ + "pointer", /* sqlite3_stmt *pStmt */ + "i32", /* int iCol */ + ], result: "f64", }, sqlite3_column_int: { - parameters: ["pointer", /* sqlite3_stmt *pStmt */ "i32" /* int iCol */], + parameters: [ + "pointer", /* sqlite3_stmt *pStmt */ + "i32", /* int iCol */ + ], result: "i32", }, sqlite3_column_int64: { - parameters: ["pointer", /* sqlite3_stmt *pStmt */ "i32" /* int iCol */], + parameters: [ + "pointer", /* sqlite3_stmt *pStmt */ + "i32", /* int iCol */ + ], result: "pointer", }, @@ -259,7 +271,10 @@ const symbols = > { }, sqlite3_column_bytes16: { - parameters: ["pointer", /* sqlite3_stmt *pStmt */ "i32" /* int iCol */], + parameters: [ + "pointer", /* sqlite3_stmt *pStmt */ + "i32", /* int iCol */ + ], result: "i32", }, @@ -277,22 +292,12 @@ const symbols = > { }, sqlite3_free: { - parameters: ["pointer"], + parameters: ["pointer" /** void* ptr */], result: "void", }, - sqlite3_blob_bytes: { - parameters: ["pointer"], - result: "i32", - }, - - sqlite3_blob_read: { - parameters: ["pointer", "pointer", "i32", "i32"], - result: "i32", - }, - sqlite3_errstr: { - parameters: ["i32"], + parameters: ["i32" /** int errcode */], result: "pointer", }, }; @@ -370,7 +375,7 @@ export function sqlite3_open_v2( const result = lib.symbols.sqlite3_open_v2( pathPtr, - Deno.UnsafePointer.of(outDB), + outDB, flags, new Deno.UnsafePointer(0n), ) as number; @@ -386,7 +391,7 @@ export function sqlite3_close_v2(handle: sqlite3) { } export function sqlite3_prepare_v3( - handle: sqlite3, + db: sqlite3, sql: string, flags = 0, ): sqlite3_stmt { @@ -396,19 +401,19 @@ export function sqlite3_prepare_v3( const outTail = new Uint8Array(8); const result = lib.symbols.sqlite3_prepare_v3( - handle, - Deno.UnsafePointer.of(sqlPtr), + db, + sqlPtr, sql.length, flags, - Deno.UnsafePointer.of(outStmt), - Deno.UnsafePointer.of(outTail), + outStmt, + outTail, ) as number; const stmt = new Deno.UnsafePointer(outStmtDV.getBigUint64(0, LITTLE_ENDIAN)); if (stmt.value === 0n && result === SQLITE3_OK) { throw new Error(`failed to prepare`); } - unwrap_error(handle, result); + unwrap_error(db, result); return stmt; } @@ -433,7 +438,7 @@ export function sqlite3_bind_text( const result = lib.symbols.sqlite3_bind_text( stmt, index, - Deno.UnsafePointer.of(value), + value, value.length, new Deno.UnsafePointer(0n), ) as number; @@ -498,7 +503,7 @@ export function sqlite3_bind_blob( const result = lib.symbols.sqlite3_bind_blob( stmt, index, - Deno.UnsafePointer.of(value), + value, value.length, new Deno.UnsafePointer(0n), ) as number; @@ -551,6 +556,7 @@ export function sqlite3_column_type(stmt: sqlite3_stmt, col: number) { export function sqlite3_column_text(stmt: sqlite3_stmt, col: number) { const ptr = lib.symbols.sqlite3_column_text(stmt, col) as Deno.UnsafePointer; + if (ptr.value === 0n) return null; return new Deno.UnsafePointerView(ptr).getCString(); } @@ -559,6 +565,7 @@ export function sqlite3_column_text16(stmt: sqlite3_stmt, col: number) { stmt, col, ) as Deno.UnsafePointer; + if (ptr.value === 0n) return null; return new Deno.UnsafePointerView(ptr).getCString(); } @@ -590,10 +597,10 @@ export function sqlite3_exec( const result = lib.symbols.sqlite3_exec( db, - Deno.UnsafePointer.of(sqlPtr), + sqlPtr, new Deno.UnsafePointer(0n), new Deno.UnsafePointer(0n), - Deno.UnsafePointer.of(outPtr), + outPtr, ); const ptr = new Deno.UnsafePointer(outDV.getBigUint64(0, LITTLE_ENDIAN)); @@ -621,7 +628,7 @@ export function sqlite3_bind_parameter_index( const namePtr = cstr(name); const index = lib.symbols.sqlite3_bind_parameter_index( stmt, - Deno.UnsafePointer.of(namePtr), + namePtr, ) as number; return index; } @@ -642,27 +649,6 @@ export function sqlite3_column_name(stmt: sqlite3_stmt, col: number) { return new Deno.UnsafePointerView(name).getCString(); } -export function sqlite3_blob_bytes(blob: sqlite3_blob) { - return lib.symbols.sqlite3_blob_bytes(blob) as number; -} - -export function sqlite3_blob_read( - blob: sqlite3_blob, - buf: Uint8Array, - offset = 0, -) { - const result = lib.symbols.sqlite3_blob_read( - blob, - Deno.UnsafePointer.of(buf), - buf.byteLength, - offset, - ) as number; - - if (result !== SQLITE3_OK) { - throw new Error(`result not ok: ${result}`); - } -} - export function sqlite3_changes(db: sqlite3) { return lib.symbols.sqlite3_changes(db) as number; } diff --git a/src/util.ts b/src/util.ts index 413606b..fa77090 100644 --- a/src/util.ts +++ b/src/util.ts @@ -1,7 +1,6 @@ export const LITTLE_ENDIAN = new Uint8Array(new Uint32Array([0x12345678]).buffer)[0] === 0x78; -// Text Encoding / Decoding Utility export function encode(str: string) { try { return (Deno as any).core.encode(str); @@ -10,29 +9,6 @@ export function encode(str: string) { } } -export function decode(bytes: Uint8Array) { - try { - return (Deno as any).core.decode(bytes); - } catch (_e) { - return new TextDecoder("utf-8").decode(bytes); - } -} - -// C String utility export function cstr(str: string) { return new Uint8Array([...encode(str), 0]); } - -export function getPlatformFileName(base: string) { - let prefix = "lib", suffix = "dll"; - - if (Deno.build.os === "windows") { - prefix = ""; - } else if (Deno.build.os === "darwin") { - suffix = "dylib"; - } else if (Deno.build.os === "linux") { - suffix = "so"; - } - - return `${prefix}${base}.${suffix}`; -} diff --git a/test/test.ts b/test/test.ts index 4d8741e..2b475fc 100644 --- a/test/test.ts +++ b/test/test.ts @@ -72,7 +72,13 @@ Deno.test("sqlite", async (t) => { ); for (let i = 0; i < 10; i++) { - stmt.execute(i, `hello ${i}`, 3.14, new Uint8Array([3, 2, 1]), null); + stmt.execute( + i, + `hello ${i}`, + 3.14, + new Uint8Array([3, 2, 1]), + null, + ); } stmt.finalize();