From 7dc1fc0195a33dc7fd2f2f16d875969fe605b824 Mon Sep 17 00:00:00 2001 From: Arun Sharma Date: Tue, 28 Apr 2026 11:43:20 -0700 Subject: [PATCH 1/4] Support standalone builds from sibling ladybug tree --- CMakeLists.txt | 47 +++++++++++++++-- README.md | 8 +++ build.js | 139 +++++++++++++++++++++++++++++++++++++++++++++---- install.js | 54 +++++++++++++++++-- 4 files changed, 231 insertions(+), 17 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 42de264..7fdffe3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,7 +1,18 @@ +cmake_minimum_required(VERSION 3.15) +project(lbugjs LANGUAGES CXX C) + +set(CMAKE_CXX_STANDARD 20) +set(CMAKE_CXX_STANDARD_REQUIRED ON) +set(CMAKE_CXX_EXTENSIONS OFF) + add_definitions(-DNAPI_VERSION=6) add_definitions(-DNODE_RUNTIME=node) add_definitions(-DBUILDING_NODE_EXTENSION) +set(LBUG_SOURCE_DIR "" CACHE PATH "Path to the Ladybug source tree used for standalone Node.js builds") +set(LBUG_BUILD_DIR "" CACHE PATH "Path to the Ladybug build tree used for standalone Node.js builds") +set(LBUG_NODEJS_EXTRA_LINK_LIBS "" CACHE STRING "Additional link libraries for standalone Node.js builds against a precompiled liblbug") + # If on Windows use npx.cmd instead of npx if(WIN32) set(NPX_CMD npx.cmd) @@ -27,6 +38,12 @@ execute_process( OUTPUT_VARIABLE CMAKE_JS_SRC ERROR_QUIET ) +execute_process( + COMMAND node -p "process.config.variables.node_prefix || ''" + WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} + OUTPUT_VARIABLE NODE_PREFIX + ERROR_QUIET +) # Without this windows breaks with: Invalid character escape '\P'. string(REPLACE "\\" "/" CMAKE_JS_INC "${CMAKE_JS_INC}") @@ -46,6 +63,7 @@ string(REPLACE "\\" "/" CMAKE_JS_SRC "${CMAKE_JS_SRC}") string(STRIP "${CMAKE_JS_INC}" CMAKE_JS_INC) string(STRIP "${CMAKE_JS_LIB}" CMAKE_JS_LIB) string(STRIP "${CMAKE_JS_SRC}" CMAKE_JS_SRC) +string(STRIP "${NODE_PREFIX}" NODE_PREFIX) # Extract last line: remove everything up to and including the last newline. string(REGEX REPLACE ".*\n" "" CMAKE_JS_INC "${CMAKE_JS_INC}") string(REGEX REPLACE ".*\n" "" CMAKE_JS_LIB "${CMAKE_JS_LIB}") @@ -61,6 +79,12 @@ endif() if(NOT CMAKE_JS_SRC MATCHES "^(/|[A-Za-z]:)") set(CMAKE_JS_SRC "") endif() +if(NOT CMAKE_JS_INC AND NODE_PREFIX MATCHES "^(/|[A-Za-z]:)") + set(CMAKE_JS_INC "${NODE_PREFIX}/include/node") +endif() +if(WIN32 AND NOT CMAKE_JS_LIB AND NODE_PREFIX MATCHES "^(/|[A-Za-z]:)") + set(CMAKE_JS_LIB "${NODE_PREFIX}/lib/node.lib") +endif() # Print CMAKE_JS variables message(STATUS "CMake.js configurations: LIB=${CMAKE_JS_LIB}, INC=${CMAKE_JS_INC}, SRC=${CMAKE_JS_SRC}") @@ -90,15 +114,29 @@ if(NOT TARGET lbug) if(NOT EXISTS "${LBUG_NODEJS_PRECOMPILED_LIB_PATH}") message(FATAL_ERROR "Precompiled liblbug archive not found: ${LBUG_NODEJS_PRECOMPILED_LIB_PATH}") endif() + + set(LBUG_NODEJS_SOURCE_INCLUDE_DIR "${PROJECT_SOURCE_DIR}/src/include") + set(LBUG_NODEJS_BUILD_INCLUDE_DIR "${PROJECT_BINARY_DIR}/src/include") + if(LBUG_SOURCE_DIR) + set(LBUG_NODEJS_SOURCE_INCLUDE_DIR "${LBUG_SOURCE_DIR}/src/include") + endif() + if(LBUG_BUILD_DIR) + set(LBUG_NODEJS_BUILD_INCLUDE_DIR "${LBUG_BUILD_DIR}/src/include") + elseif(LBUG_SOURCE_DIR) + set(LBUG_NODEJS_BUILD_INCLUDE_DIR "${LBUG_SOURCE_DIR}/build/release/src/include") + endif() + add_library(lbug STATIC IMPORTED GLOBAL) set_target_properties(lbug PROPERTIES IMPORTED_LOCATION "${LBUG_NODEJS_PRECOMPILED_LIB_PATH}") target_include_directories(lbug INTERFACE - "${PROJECT_SOURCE_DIR}/src/include" - "${PROJECT_BINARY_DIR}/src/include") + "${LBUG_NODEJS_SOURCE_INCLUDE_DIR}" + "${LBUG_NODEJS_BUILD_INCLUDE_DIR}") if(WIN32) target_compile_definitions(lbug INTERFACE LBUG_STATIC_DEFINE) endif() - set(NODEJS_LBUG_LINK_DEPS "$") + if(TARGET lbug_link_deps) + set(NODEJS_LBUG_LINK_DEPS "$") + endif() endif() add_library(lbugjs SHARED ${CPP_SOURCE_FILES}) @@ -117,3 +155,6 @@ else() target_link_options(lbugjs PRIVATE -Wl,--export-dynamic) endif() target_link_libraries(lbugjs PRIVATE lbug ${NODEJS_LBUG_LINK_DEPS} ${CMAKE_JS_LIB}) +if(LBUG_NODEJS_EXTRA_LINK_LIBS) + target_link_libraries(lbugjs PRIVATE ${LBUG_NODEJS_EXTRA_LINK_LIBS}) +endif() diff --git a/README.md b/README.md index 086c63e..5633336 100644 --- a/README.md +++ b/README.md @@ -90,6 +90,14 @@ npm install --include=dev npm run build ``` +By default, the standalone build looks for a sibling Ladybug checkout at +`../ladybug` via `LBUG_SOURCE_DIR`, and reuses the prebuilt static library from +`../ladybug/build/release`. Override that location like this: + +```bash +LBUG_SOURCE_DIR=/path/to/ladybug npm run build +``` + ### Run Tests ```bash diff --git a/build.js b/build.js index 2c7c29f..71f30b6 100644 --- a/build.js +++ b/build.js @@ -1,29 +1,146 @@ const fs = require("fs"); +const os = require("os"); const path = require("path"); -const { execSync } = require("child_process"); +const { execFileSync, execSync } = require("child_process"); -const SRC_PATH = path.resolve(__dirname, "../.."); -const THREADS = require("os").cpus().length; +const THREADS = os.cpus().length; +const DEFAULT_LBUG_SOURCE_DIR = path.resolve(__dirname, "../ladybug"); + +function resolveExistingPath(candidate) { + if (!candidate) { + return null; + } + const resolved = path.resolve(candidate); + return fs.existsSync(resolved) ? resolved : null; +} + +function getDefaultBuildDir(lbugSourceDir) { + return lbugSourceDir ? path.join(lbugSourceDir, "build", "release") : null; +} + +function getDefaultPrecompiledLibPath(lbugBuildDir) { + if (!lbugBuildDir) { + return null; + } + const candidates = process.platform === "win32" + ? [ + path.join(lbugBuildDir, "src", "Release", "lbug.lib"), + path.join(lbugBuildDir, "src", "lbug.lib"), + path.join(lbugBuildDir, "src", "Release", "liblbug.lib"), + path.join(lbugBuildDir, "src", "liblbug.lib"), + ] + : [path.join(lbugBuildDir, "src", "liblbug.a")]; + return candidates.find((candidate) => fs.existsSync(candidate)) || null; +} + +function getDefaultExtraLinkLibs(lbugBuildDir, precompiledLibPath) { + if (!lbugBuildDir || !precompiledLibPath) { + return []; + } + + const linkTxtCandidates = [ + path.join(lbugBuildDir, "tools", "python_api", "CMakeFiles", "_lbug.dir", "link.txt"), + path.join(lbugBuildDir, "src", "CMakeFiles", "lbug_shared.dir", "link.txt"), + ]; + const tokenPattern = /"[^"]+"|\S+/g; + + for (const linkTxtPath of linkTxtCandidates) { + if (!fs.existsSync(linkTxtPath)) { + continue; + } + const linkTxtDir = path.dirname(linkTxtPath); + const linkBaseDir = linkTxtPath.includes(`${path.sep}tools${path.sep}python_api${path.sep}`) + ? path.join(lbugBuildDir, "tools", "python_api") + : linkTxtPath.includes(`${path.sep}src${path.sep}CMakeFiles${path.sep}`) + ? path.join(lbugBuildDir, "src") + : linkTxtDir; + const tokens = fs.readFileSync(linkTxtPath, "utf8").match(tokenPattern) || []; + const libs = []; + let sawPrecompiledLib = false; + + for (const rawToken of tokens) { + const token = rawToken.replace(/^"(.*)"$/, "$1"); + const resolvedToken = token.startsWith("-") + ? token + : fs.existsSync(path.resolve(linkBaseDir, token)) + ? path.resolve(linkBaseDir, token) + : path.resolve(linkTxtDir, token); + + if (!sawPrecompiledLib) { + if (resolvedToken === precompiledLibPath) { + sawPrecompiledLib = true; + } + continue; + } + + if (token.startsWith("-l")) { + libs.push(token); + continue; + } + if (!/\.(a|lib|dylib|so|tbd)$/.test(token)) { + continue; + } + if (resolvedToken === precompiledLibPath) { + continue; + } + libs.push(resolvedToken); + } + + if (libs.length > 0) { + return [...new Set(libs)]; + } + } + + return []; +} console.log(`Using ${THREADS} threads to build Lbug.`); const env = { ...process.env }; -const precompiledLibPath = env.LBUG_NODEJS_PRECOMPILED_LIB_PATH; +const lbugSourceDir = resolveExistingPath(env.LBUG_SOURCE_DIR || DEFAULT_LBUG_SOURCE_DIR); +const lbugBuildDir = resolveExistingPath(env.LBUG_BUILD_DIR || getDefaultBuildDir(lbugSourceDir)); +const precompiledLibPath = resolveExistingPath( + env.LBUG_NODEJS_PRECOMPILED_LIB_PATH || getDefaultPrecompiledLibPath(lbugBuildDir) +); +const extraLinkLibs = getDefaultExtraLinkLibs(lbugBuildDir, precompiledLibPath); + +if (lbugSourceDir) { + env.LBUG_SOURCE_DIR = lbugSourceDir; +} +if (lbugBuildDir) { + env.LBUG_BUILD_DIR = lbugBuildDir; +} + +const cmakeArgs = env.EXTRA_CMAKE_FLAGS + ? env.EXTRA_CMAKE_FLAGS.trim().split(/\s+/).filter(Boolean) + : []; +if (lbugSourceDir) { + cmakeArgs.push(`-DLBUG_SOURCE_DIR=${lbugSourceDir}`); +} +if (lbugBuildDir) { + cmakeArgs.push(`-DLBUG_BUILD_DIR=${lbugBuildDir}`); +} if (precompiledLibPath) { - const extraFlags = [ - env.EXTRA_CMAKE_FLAGS, + cmakeArgs.push( "-DBUILD_LBUG=FALSE", "-DBUILD_SHELL=FALSE", "-DLBUG_NODEJS_USE_PRECOMPILED_LIB=TRUE", - `-DLBUG_NODEJS_PRECOMPILED_LIB_PATH=${precompiledLibPath}`, - ].filter(Boolean).join(" "); - env.EXTRA_CMAKE_FLAGS = extraFlags; + `-DLBUG_NODEJS_PRECOMPILED_LIB_PATH=${precompiledLibPath}` + ); console.log(`Using precompiled liblbug from ${precompiledLibPath}.`); } +if (extraLinkLibs.length > 0) { + cmakeArgs.push(`-DLBUG_NODEJS_EXTRA_LINK_LIBS=${extraLinkLibs.join(";")}`); +} execSync("npm run clean", { stdio: "inherit" }); -execSync(`make nodejs NUM_THREADS=${THREADS}`, { - cwd: SRC_PATH, +execFileSync("cmake", ["-S", ".", "-B", "build", ...cmakeArgs], { + cwd: __dirname, + env, + stdio: "inherit", +}); +execFileSync("cmake", ["--build", "build", "-j", `${THREADS}`], { + cwd: __dirname, env, stdio: "inherit", }); diff --git a/install.js b/install.js index 358239f..cc11652 100644 --- a/install.js +++ b/install.js @@ -7,6 +7,34 @@ const process = require("process"); const isNpmBuildFromSourceSet = process.env.npm_config_build_from_source; const platform = process.platform; const arch = process.arch; +const DEFAULT_LBUG_SOURCE_DIR = path.resolve(__dirname, "../ladybug"); + +function resolveExistingPath(candidate) { + if (!candidate) { + return null; + } + const resolved = path.resolve(candidate); + return fs.existsSync(resolved) ? resolved : null; +} + +function getDefaultBuildDir(lbugSourceDir) { + return lbugSourceDir ? path.join(lbugSourceDir, "build", "release") : null; +} + +function getDefaultPrecompiledLibPath(lbugBuildDir) { + if (!lbugBuildDir) { + return null; + } + const candidates = platform === "win32" + ? [ + path.join(lbugBuildDir, "src", "Release", "lbug.lib"), + path.join(lbugBuildDir, "src", "lbug.lib"), + path.join(lbugBuildDir, "src", "Release", "liblbug.lib"), + path.join(lbugBuildDir, "src", "liblbug.lib"), + ] + : [path.join(lbugBuildDir, "src", "liblbug.a")]; + return candidates.find((candidate) => fs.existsSync(candidate)) || null; +} // When the package was published with prebuilt binaries, each platform's // binary lives in a dedicated optional sub-package. npm may hoist it to the @@ -59,6 +87,17 @@ if (isNpmBuildFromSourceSet) { console.log("Prebuilt binary is not available, building from source..."); } +const externalLbugSourceDir = resolveExistingPath( + process.env.LBUG_SOURCE_DIR || DEFAULT_LBUG_SOURCE_DIR +); +const externalLbugBuildDir = resolveExistingPath( + process.env.LBUG_BUILD_DIR || getDefaultBuildDir(externalLbugSourceDir) +); +const externalPrecompiledLibPath = resolveExistingPath( + process.env.LBUG_NODEJS_PRECOMPILED_LIB_PATH || + getDefaultPrecompiledLibPath(externalLbugBuildDir) +); + // Get number of threads const THREADS = os.cpus().length; console.log(`Using ${THREADS} threads to build Lbug.`); @@ -73,6 +112,12 @@ childProcess.execSync("npm install", { // Build the Lbug source code console.log("Building Lbug source code..."); const env = { ...process.env }; +if (externalLbugSourceDir) { + env.LBUG_SOURCE_DIR = externalLbugSourceDir; +} +if (externalLbugBuildDir) { + env.LBUG_BUILD_DIR = externalLbugBuildDir; +} if (process.platform === "darwin") { const archflags = process.env["ARCHFLAGS"] @@ -128,16 +173,19 @@ if (process.platform === "win32") { ); } -if (env.LBUG_NODEJS_PRECOMPILED_LIB_PATH) { +if (externalPrecompiledLibPath) { + env.LBUG_NODEJS_PRECOMPILED_LIB_PATH = externalPrecompiledLibPath; env.EXTRA_CMAKE_FLAGS = [ env.EXTRA_CMAKE_FLAGS, + env.LBUG_SOURCE_DIR ? `-DLBUG_SOURCE_DIR=${env.LBUG_SOURCE_DIR}` : null, + env.LBUG_BUILD_DIR ? `-DLBUG_BUILD_DIR=${env.LBUG_BUILD_DIR}` : null, "-DBUILD_LBUG=FALSE", "-DBUILD_SHELL=FALSE", "-DLBUG_NODEJS_USE_PRECOMPILED_LIB=TRUE", - `-DLBUG_NODEJS_PRECOMPILED_LIB_PATH=${env.LBUG_NODEJS_PRECOMPILED_LIB_PATH}`, + `-DLBUG_NODEJS_PRECOMPILED_LIB_PATH=${externalPrecompiledLibPath}`, ].filter(Boolean).join(" "); console.log( - `Using precompiled liblbug from '${env.LBUG_NODEJS_PRECOMPILED_LIB_PATH}'.` + `Using precompiled liblbug from '${externalPrecompiledLibPath}'.` ); } From 0de12bb6bc81f67607465d2ce710c368e47ef19a Mon Sep 17 00:00:00 2001 From: Arun Sharma Date: Tue, 28 Apr 2026 11:44:51 -0700 Subject: [PATCH 2/4] Add dataset submodule for standalone tests --- .gitmodules | 3 +++ README.md | 1 + dataset | 1 + test/common.js | 13 ++++++++++--- 4 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 .gitmodules create mode 160000 dataset diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 0000000..2638e6f --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule "dataset"] + path = dataset + url = https://github.com/ladybugdb/dataset.git diff --git a/README.md b/README.md index 5633336..42b8963 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,7 @@ Both CommonJS (`require`) and ES Modules (`import`) are fully supported. ### Install Dev Dependencies ```bash +git submodule update --init --recursive npm install --include=dev ``` diff --git a/dataset b/dataset new file mode 160000 index 0000000..5553111 --- /dev/null +++ b/dataset @@ -0,0 +1 @@ +Subproject commit 55531118c5e0c683fc3a3d806b7abd0b09a31ff8 diff --git a/test/common.js b/test/common.js index 433ca94..edc4c9e 100644 --- a/test/common.js +++ b/test/common.js @@ -18,6 +18,9 @@ if (TEST_INSTALLED) { const tmp = require("tmp"); const fs = require("fs/promises"); const path = require("path"); + +const toCypherPath = (filePath) => filePath.replace(/\\/g, "/"); + const initTests = async () => { const tmpPath = await new Promise((resolve, reject) => { tmp.dir({ unsafeCleanup: true }, (err, path, _) => { @@ -31,7 +34,11 @@ const initTests = async () => { const dbPath = path.join(tmpPath, "db.lbdb"); const db = new lbug.Database(dbPath, 1 << 28 /* 256MB */); const conn = new lbug.Connection(db, 4); - const tinysnbDir = "../../dataset/tinysnb/"; + const datasetDir = path.resolve(__dirname, "..", "dataset"); + const tinysnbDir = path.join(datasetDir, "tinysnb") + path.sep; + const tinysnbSerialDir = path.join(datasetDir, "tinysnb-serial") + path.sep; + const cypherTinysnbDir = toCypherPath(tinysnbDir); + const cypherTinysnbSerialDir = toCypherPath(tinysnbSerialDir); const schema = (await fs.readFile(tinysnbDir + "schema.cypher")) .toString() @@ -56,7 +63,7 @@ const initTests = async () => { } // handle multiple data files in one line - const statement = line.replace(dataFileRegex, `"${tinysnbDir}$1"`); + const statement = line.replace(dataFileRegex, `"${cypherTinysnbDir}$1"`); await conn.query(statement); } @@ -65,7 +72,7 @@ const initTests = async () => { "create node table moviesSerial (ID SERIAL, name STRING, length INT32, note STRING, PRIMARY KEY (ID))" ); await conn.query( - 'copy moviesSerial from "../../dataset/tinysnb-serial/vMovies.csv"' + `copy moviesSerial from "${cypherTinysnbSerialDir}vMovies.csv"` ); global.dbPath = dbPath; From 762e3bb7f139b331a87f23dd4d7a624b21821690 Mon Sep 17 00:00:00 2001 From: Arun Sharma Date: Mon, 27 Apr 2026 20:11:46 -0700 Subject: [PATCH 3/4] Keep query results alive through connection ownership NodeQueryResult now retains the native Connection instead of a Database handle. The result objects depend on database-owned state, and the native connection is the stable owner of that state even after the JS connection wrapper is closed. This fixes the case where async query results are discarded and later finalized after conn.closeSync() and db.closeSync(). The finalizer can still destroy the MaterializedQueryResult safely because the retained connection keeps the backing database state alive. Also add a regression test that mirrors the file-backed async query sequence from the external crash reproducer. --- src_cpp/include/node_connection.h | 8 ++++---- src_cpp/include/node_query_result.h | 8 +++++--- src_cpp/node_connection.cpp | 7 ++++--- src_cpp/node_query_result.cpp | 15 ++++++++++----- test/test_database.js | 27 +++++++++++++++++++++++++-- 5 files changed, 48 insertions(+), 17 deletions(-) diff --git a/src_cpp/include/node_connection.h b/src_cpp/include/node_connection.h index a47be64..7406a40 100644 --- a/src_cpp/include/node_connection.h +++ b/src_cpp/include/node_connection.h @@ -105,7 +105,7 @@ class ConnectionExecuteAsyncWorker : public Napi::AsyncWorker { SetError(result->getErrorMessage()); return; } - nodeQueryResult->AdoptQueryResult(std::move(result), database); + nodeQueryResult->AdoptQueryResult(std::move(result), connection, database); } catch (const std::exception& exc) { SetError(std::string(exc.what())); } @@ -132,8 +132,8 @@ class ConnectionExecuteAsyncWorker : public Napi::AsyncWorker { class ConnectionQueryAsyncWorker : public Napi::AsyncWorker { public: ConnectionQueryAsyncWorker(Napi::Function& callback, std::shared_ptr& connection, - std::shared_ptr& database, - std::string statement, NodeQueryResult* nodeQueryResult, Napi::Value progressCallback) + std::shared_ptr& database, std::string statement, + NodeQueryResult* nodeQueryResult, Napi::Value progressCallback) : Napi::AsyncWorker(callback), connection(connection), database(database), statement(std::move(statement)), nodeQueryResult(nodeQueryResult) { if (progressCallback.IsFunction()) { @@ -162,7 +162,7 @@ class ConnectionQueryAsyncWorker : public Napi::AsyncWorker { SetError(result->getErrorMessage()); return; } - nodeQueryResult->AdoptQueryResult(std::move(result), database); + nodeQueryResult->AdoptQueryResult(std::move(result), connection, database); } catch (const std::exception& exc) { SetError(std::string(exc.what())); } diff --git a/src_cpp/include/node_query_result.h b/src_cpp/include/node_query_result.h index 07d1345..b300eb7 100644 --- a/src_cpp/include/node_query_result.h +++ b/src_cpp/include/node_query_result.h @@ -21,9 +21,10 @@ class NodeQueryResult : public Napi::ObjectWrap { public: static Napi::Object Init(Napi::Env env, Napi::Object exports); static Napi::Object NewInstance(Napi::Env env, std::unique_ptr queryResult, - std::shared_ptr db); + std::shared_ptr connection, std::shared_ptr database); explicit NodeQueryResult(const Napi::CallbackInfo& info); - void AdoptQueryResult(std::unique_ptr queryResult, std::shared_ptr db); + void AdoptQueryResult(std::unique_ptr queryResult, + std::shared_ptr connection, std::shared_ptr database); std::unique_ptr DetachNextQueryResult(); ~NodeQueryResult() override; @@ -53,6 +54,7 @@ class NodeQueryResult : public Napi::ObjectWrap { private: static Napi::FunctionReference constructor; std::unique_ptr ownedQueryResult = nullptr; + std::shared_ptr connection = nullptr; std::shared_ptr database = nullptr; std::unique_ptr> columnNames = nullptr; std::atomic activeAsyncUses = 0; @@ -205,7 +207,7 @@ class NodeQueryResultGetNextQueryResultAsyncWorker : public Napi::AsyncWorker { return; } Callback().Call({env.Null(), NodeQueryResult::NewInstance(env, std::move(nextOwnedResult), - currQueryResult->database)}); + currQueryResult->connection, currQueryResult->database)}); } void OnError(Napi::Error const& error) override { diff --git a/src_cpp/node_connection.cpp b/src_cpp/node_connection.cpp index c093fbf..8857666 100644 --- a/src_cpp/node_connection.cpp +++ b/src_cpp/node_connection.cpp @@ -116,7 +116,7 @@ Napi::Value NodeConnection::QuerySync(const Napi::CallbackInfo& info) { if (!result->isSuccess()) { Napi::Error::New(env, result->getErrorMessage()).ThrowAsJavaScriptException(); } - nodeQueryResult->AdoptQueryResult(std::move(result), database); + nodeQueryResult->AdoptQueryResult(std::move(result), connection, database); } catch (const std::exception& exc) { Napi::Error::New(env, std::string(exc.what())).ThrowAsJavaScriptException(); } @@ -136,7 +136,7 @@ Napi::Value NodeConnection::ExecuteSync(const Napi::CallbackInfo& info) { if (!result->isSuccess()) { Napi::Error::New(env, result->getErrorMessage()).ThrowAsJavaScriptException(); } - nodeQueryResult->AdoptQueryResult(std::move(result), database); + nodeQueryResult->AdoptQueryResult(std::move(result), connection, database); } catch (const std::exception& exc) { Napi::Error::New(env, std::string(exc.what())).ThrowAsJavaScriptException(); } @@ -150,7 +150,8 @@ Napi::Value NodeConnection::QueryAsync(const Napi::CallbackInfo& info) { auto nodeQueryResult = Napi::ObjectWrap::Unwrap(info[1].As()); auto callback = info[2].As(); auto asyncWorker = - new ConnectionQueryAsyncWorker(callback, connection, database, statement, nodeQueryResult, info[3]); + new ConnectionQueryAsyncWorker( + callback, connection, database, statement, nodeQueryResult, info[3]); asyncWorker->Queue(); return info.Env().Undefined(); } diff --git a/src_cpp/node_query_result.cpp b/src_cpp/node_query_result.cpp index 2d8f854..931b5c5 100644 --- a/src_cpp/node_query_result.cpp +++ b/src_cpp/node_query_result.cpp @@ -37,10 +37,12 @@ Napi::Object NodeQueryResult::Init(Napi::Env env, Napi::Object exports) { } Napi::Object NodeQueryResult::NewInstance( - Napi::Env /*env*/, std::unique_ptr queryResult, std::shared_ptr db) { + Napi::Env /*env*/, std::unique_ptr queryResult, + std::shared_ptr connection, std::shared_ptr database) { auto obj = constructor.New({}); auto* nodeQueryResult = Napi::ObjectWrap::Unwrap(obj); - nodeQueryResult->AdoptQueryResult(std::move(queryResult), std::move(db)); + nodeQueryResult->AdoptQueryResult( + std::move(queryResult), std::move(connection), std::move(database)); return obj; } @@ -52,11 +54,13 @@ NodeQueryResult::~NodeQueryResult() { } void NodeQueryResult::AdoptQueryResult( - std::unique_ptr queryResult, std::shared_ptr db) { + std::unique_ptr queryResult, std::shared_ptr connection, + std::shared_ptr database) { ThrowIfAsyncOperationInFlight("replace"); columnNames.reset(); ownedQueryResult = std::move(queryResult); - database = std::move(db); + this->connection = std::move(connection); + this->database = std::move(database); } std::unique_ptr NodeQueryResult::DetachNextQueryResult() { @@ -142,7 +146,7 @@ Napi::Value NodeQueryResult::GetNextQueryResultSync(const Napi::CallbackInfo& in .ThrowAsJavaScriptException(); return env.Undefined(); } - return NewInstance(env, std::move(nextOwnedResult), database); + return NewInstance(env, std::move(nextOwnedResult), connection, database); } catch (const std::exception& exc) { Napi::Error::New(env, std::string(exc.what())).ThrowAsJavaScriptException(); } @@ -288,5 +292,6 @@ void NodeQueryResult::Close(const Napi::CallbackInfo& info) { void NodeQueryResult::Close() { columnNames.reset(); ownedQueryResult.reset(); + connection.reset(); database.reset(); } diff --git a/test/test_database.js b/test/test_database.js index 916ba84..4717d2b 100644 --- a/test/test_database.js +++ b/test/test_database.js @@ -518,8 +518,9 @@ describe("Database close", function () { // MaterializedQueryResult whose FactorizedTable destructor accesses // database-owned memory. If the Database is destroyed before the GC // finalizer for NodeQueryResult runs, that destructor crashes. The fix - // is for NodeQueryResult to hold a shared_ptr so the Database - // cannot be freed until every result that references it is gone. + // is for NodeQueryResult to hold a shared_ptr so the native + // connection keeps the database-backed state alive until every result + // that references it is gone. // // The key pattern being tested is: query results are *not stored*, making // them immediately eligible for GC. conn.closeSync() and db.closeSync() @@ -536,4 +537,26 @@ describe("Database close", function () { assert.isTrue(conn._isClosed); assert.isTrue(testDb._isClosed); }); + + it("should not crash when file-backed async query results are discarded before close", async function () { + const tmpDbPath = await new Promise((resolve, reject) => { + tmp.dir({ unsafeCleanup: true }, (err, path, _) => { + if (err) { + return reject(err); + } + return resolve(path); + }); + }); + const dbPath = path.join(tmpDbPath, "test.lbdb"); + const testDb = new lbug.Database(dbPath); + const conn = new lbug.Connection(testDb); + await conn.query("CREATE NODE TABLE IF NOT EXISTS T(id STRING PRIMARY KEY)"); + const id = `test-${Date.now()}`; + await conn.query(`CREATE (:T {id: '${id}'})`); + await conn.query("MATCH (t:T) RETURN t.id"); + conn.closeSync(); + testDb.closeSync(); + assert.isTrue(conn._isClosed); + assert.isTrue(testDb._isClosed); + }); }); From b8f0a3cda1db79ad707b774e9bd42ad318718d29 Mon Sep 17 00:00:00 2001 From: Arun Sharma Date: Tue, 28 Apr 2026 12:45:57 -0700 Subject: [PATCH 4/4] ci: use submodules and ccache --- .github/workflows/ci.yml | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4e87862..739cf9d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,7 +34,9 @@ jobs: - name: Checkout nodejs_api uses: actions/checkout@v4 with: + fetch-depth: 1 path: tools/nodejs_api + submodules: recursive - name: Fetch ladybug source archive shell: bash @@ -45,13 +47,16 @@ jobs: # it into the working directory, excluding tools/nodejs_api since we # already have it checked out there. - - name: Fetch dataset archive - shell: bash - run: | - mkdir -p dataset - curl -fsSL https://github.com/ladybugdb/dataset/archive/refs/heads/main.tar.gz \ - | tar -xz --strip-components=1 -C dataset - # Tests reference ../../dataset/ relative to tools/nodejs_api. + - name: Setup ccache (non-Windows) + if: ${{ matrix.platform != 'win32' }} + uses: hendrikmuhs/ccache-action@v1.2 + with: + key: nodejs-${{ runner.os }}-${{ runner.arch }}-${{ github.ref }} + max-size: 2G + create-symlink: true + restore-keys: | + nodejs-${{ runner.os }}-${{ runner.arch }}-refs/heads/main + nodejs-${{ runner.os }}-${{ runner.arch }}- - name: Install Node.js dependencies (non-Windows) if: ${{ matrix.platform != 'win32' }} @@ -117,6 +122,8 @@ jobs: env: MACOSX_DEPLOYMENT_TARGET: ${{ matrix.mac_env && '13.3' || '' }} CMAKE_OSX_ARCHITECTURES: ${{ matrix.mac_env && matrix.arch || '' }} + CMAKE_C_COMPILER_LAUNCHER: ccache + CMAKE_CXX_COMPILER_LAUNCHER: ccache - name: Run Node.js tests working-directory: tools/nodejs_api