From 7f8741cbd7c3460336d48817f8d6321ace0bd5a5 Mon Sep 17 00:00:00 2001 From: wudidapaopao Date: Tue, 3 Nov 2026 17:10:56 +0000 Subject: [PATCH 1/7] Move the engine pin from v26.7.3 to v26.11.0-rc.1 Pin chdb-core v26.11.0-rc.1 (ClickHouse master def2fdf4caa, 26.11.1.1) to validate the release candidate before the full v26.11.1 release. This is the same edit the engine release check makes: CHDB_ENGINE_PIN, and LIBCHDB_NPM_VERSION plus the four @chdb/lib-* optionalDependencies moved to 26.11.0-rc.1.1, the scheme's first packaging of an rc engine. The publish workflow already routes an -rc. subpackage to the rc dist-tag. The places that name the pin in prose move with it: the README's durable section, the test workflow header and the design doc's still-to-do item. The update_libchdb.sh comment still described 26.7.2-rc.2 and the old exact-match rule; it now names this build's chdb_version() string and the min_reader floor that replaced exact matching. The C ABI change since v26.7.3 is additive (chdb_insert_arrow_array and chdb_insert_arrow_stream, unused here) plus documentation, so the addon needs no change. --- .github/workflows/chdb-node-test.yml | 7 ++++--- README.md | 2 +- docs/design/durable-control-plane.md | 2 +- package.json | 8 ++++---- update_libchdb.sh | 19 ++++++++++--------- 5 files changed, 20 insertions(+), 18 deletions(-) diff --git a/.github/workflows/chdb-node-test.yml b/.github/workflows/chdb-node-test.yml index 0364900..a5f43e9 100644 --- a/.github/workflows/chdb-node-test.yml +++ b/.github/workflows/chdb-node-test.yml @@ -10,9 +10,10 @@ on: # update_libchdb.sh pins a published chdb-core libchdb across all four target # platforms, so the full v2 + v3 suite is expected green here. The pin is -# currently v26.7.3, the first stable release to export the Durable V1 ABI -# (chdb_backup_database_n / chdb_restore_database_n / chdb_classify_query_n); -# v26.7.2-rc.2 was the candidate that introduced them. +# currently the v26.11.0-rc.1 release candidate. v26.7.3 was the first stable +# release to export the Durable V1 ABI (chdb_backup_database_n / +# chdb_restore_database_n / chdb_classify_query_n); v26.7.2-rc.2 was the +# candidate that introduced them. # Harden the registry fetch so a transient reset (e.g. ECONNRESET during # install) retries instead of reddening the job. Combined with the per-job diff --git a/README.md b/README.md index 98358e3..d36c66d 100644 --- a/README.md +++ b/README.md @@ -200,7 +200,7 @@ and the sync policy with `@clickhouse/client`. > is injected; Node callers get a default engine over this package's own addon > from `chdb/durable/node`. > -> Requires an engine exporting the durable ABI — currently `26.7.3`. +> Requires an engine exporting the durable ABI — currently `26.11.0-rc.1`. > Compatibility is a floor rather than an equality: an object records > `min_reader` and `backup_format`, and any engine at or above that floor opens > it. An object written by `26.7.2-rc.2` stays readable on `26.7.3` and later. diff --git a/docs/design/durable-control-plane.md b/docs/design/durable-control-plane.md index 710477d..4a3775c 100644 --- a/docs/design/durable-control-plane.md +++ b/docs/design/durable-control-plane.md @@ -724,7 +724,7 @@ this package: 1. Publish `@chdb/lib-` packages built from a tree that binds the durable ABI. `optionalDependencies` and `update_libchdb.sh` already name - `26.7.3-stable.1`, and the release workflow derives the subpackage version + `26.11.0-rc.1.1`, and the release workflow derives the subpackage version from the latter — but nothing has been tagged since, so that version is not on npm at all. The newest published is `26.7.0-stable.1` (main `chdb@3.3.0` pins it), which carries an addon from before this work and exports none of diff --git a/package.json b/package.json index ad849c9..67c32bf 100644 --- a/package.json +++ b/package.json @@ -121,10 +121,10 @@ "node-gyp-build": "^4.6.0" }, "optionalDependencies": { - "@chdb/lib-darwin-arm64": "26.7.3-stable.1", - "@chdb/lib-darwin-x64": "26.7.3-stable.1", - "@chdb/lib-linux-arm64-gnu": "26.7.3-stable.1", - "@chdb/lib-linux-x64-gnu": "26.7.3-stable.1" + "@chdb/lib-darwin-arm64": "26.11.0-rc.1.1", + "@chdb/lib-darwin-x64": "26.11.0-rc.1.1", + "@chdb/lib-linux-arm64-gnu": "26.11.0-rc.1.1", + "@chdb/lib-linux-x64-gnu": "26.11.0-rc.1.1" }, "peerDependencies": { "@clickhouse/client": ">=1.23.0", diff --git a/update_libchdb.sh b/update_libchdb.sh index 2dd8753..0bc3c8a 100755 --- a/update_libchdb.sh +++ b/update_libchdb.sh @@ -11,20 +11,21 @@ cd "$(dirname "$0")" # Fail fast so a bad download never silently leaves a stale/partial libchdb. set -e -# Pre-release engine carrying the Durable V1 ABI: chdb_backup_database_n, +# The engine has to export the Durable V1 ABI: chdb_backup_database_n, # chdb_restore_database_n and the chdb_classify_query_n that reports statement # count and target-database proof. chdb/durable's engine adapter cannot be built -# without them, and no stable release exports them yet. +# without them. v26.7.3 is the first stable release that exports them. # -# Note chdb_version() on this build returns the full "26.7.2-rc.2", suffix and -# all. Durable records that string in head.engine.version and opens only on an -# exact match, so an object written against this RC is not readable by the -# eventual 26.7.2 stable. That is the contract working as designed (V1 pins the -# engine exactly), but it means RC-era objects are throwaway. +# The pin is currently a release candidate, and chdb_version() on it returns the +# full "26.11.0-rc.1", suffix and all. Durable records that string in +# head.engine.version and raises min_reader to it on every write. A pre-release +# sorts below the release it leads to, so an object written against this RC +# stays readable by the eventual stable, but every earlier engine (26.9.x and +# before) is refused it with engine_incompatible from then on. # # Keep the pin on its own line and literal: the release check greps for it when # it proposes a bump. -CHDB_ENGINE_PIN=v26.7.3 +CHDB_ENGINE_PIN=v26.11.0-rc.1 # CHDB_ENGINE_VERSION overrides the pin, which is how the release check runs the # suite against an engine this repository has not adopted yet. It is a different @@ -52,7 +53,7 @@ LATEST_RELEASE="${CHDB_ENGINE_VERSION:-$CHDB_ENGINE_PIN}" # ERR_DLOPEN_FAILED instead. # # The publish and cleanroom workflows read CHDB_LIB_VERSION from here. -LIBCHDB_NPM_VERSION=26.7.3-stable.1 +LIBCHDB_NPM_VERSION=26.11.0-rc.1.1 # Download the correct version based on the platform case "$(uname -s)" in From e43dde92cdc27d26b7928cf06834c6a5b9ee9a2d Mon Sep 17 00:00:00 2001 From: wudidapaopao Date: Tue, 3 Nov 2026 17:10:56 +0000 Subject: [PATCH 2/7] Skip the four parity cases that moved with the ClickHouse 26.9 baseline The parity runner runs @clickhouse/client 1.23.0's integration suite, which predates two server changes that every chdb-core engine from v26.9.0 on carries (v26.7.3 passes all four): - Analyzer errors now read "identifier `x`. In scope ..." with a capital I, so the tag's case-sensitive "in scope" regex misses. Code and type still parse correctly. - An epoch-milliseconds JSON number inserted into DateTime64(3) saturates to 9999-12-31 23:59:59.000. A JSON string still parses, and chdb-node's own serializers always send Date values as strings. ClickHouse/clickhouse-js#1025 (in client-1.24.0) relaxed the regex and skips the DateTime64 cases on servers that clamp. Moving the runner to 1.24.x is a separate sync: it fixes these four but brings three failures of its own that fail on v26.7.3 as well. Until then, skip these with the reason recorded, and drop them when syncedAgainst moves. --- tests/clickhouse-js/skip_list.json | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/clickhouse-js/skip_list.json b/tests/clickhouse-js/skip_list.json index 02a2167..31ff365 100644 --- a/tests/clickhouse-js/skip_list.json +++ b/tests/clickhouse-js/skip_list.json @@ -226,6 +226,26 @@ "file": "packages/client-node/__tests__/integration/node_streaming_e2e.test.ts", "test": "[Node.js] streaming e2e > should stream a Parquet file", "reason": "Streams a Parquet file into chdb via `INSERT FORMAT Parquet`; chdb's `ParquetV3BlockInputFormat` rejects this specific fixture with `apache::thrift::protocol::TProtocolException: invalid TType`. The server's older Parquet input format accepts it. Engine-version divergence in Parquet decoder." + }, + { + "file": "packages/client-common/__tests__/integration/error_parsing.test.ts", + "test": "ClickHouse server errors parsing > returns \"unknown identifier\" error", + "reason": "Engine-version divergence, not a chdb gap. Since chdb-core v26.9.0 (ClickHouse 26.9.2.1; v26.7.3 is unaffected) the analyzer's message ends the identifier with `. In scope ...` (capital I) where this tag's case-sensitive regex expects ` in scope`. Code 47 and type UNKNOWN_IDENTIFIER still parse correctly; only the message match misses. Upstream accepts both spellings since ClickHouse/clickhouse-js#1025 (client-1.24.0). Drop this entry when syncedAgainst moves to that release or later." + }, + { + "file": "packages/client-common/__tests__/integration/error_parsing.test.ts", + "test": "ClickHouse server errors parsing > returns \"unknown table\" error", + "reason": "Same `. In scope` message change as the entry above. Code 60 and type UNKNOWN_TABLE still parse correctly. Fixed upstream by the same clickhouse-js#1025." + }, + { + "file": "packages/client-common/__tests__/integration/date_time.test.ts", + "test": "DateTime > DateTime64(3) > should insert DateTime64(3) and get it back (different timezone)", + "reason": "Engine-version divergence, not a chdb gap. Since chdb-core v26.9.0 (ClickHouse 26.9.2.1; v26.7.3 is unaffected) an epoch-milliseconds JSON number inserted into DateTime64(3) saturates to 9999-12-31 23:59:59.000 instead of being read as ticks; the same value sent as a JSON string still parses. chdb-node never sends a Date this way (it formats Date values as strings), so this only reaches callers who put raw epoch-ms numbers into JSONEachRow themselves. Upstream skips this test on such servers since clickhouse-js#1025 (isDateTime64FromJSONNumberBroken)." + }, + { + "file": "packages/client-common/__tests__/integration/data_types.test.ts", + "test": "data types > should work with custom JSON handling (BigInt and Date)", + "reason": "Same epoch-ms DateTime64(3) saturation as the entry above: the test's custom JSON.stringify sends a Date as getTime(). Upstream skips it on such servers since clickhouse-js#1025." } ] } From 5135c037b2e735efb75faaa0c025fa942563b64f Mon Sep 17 00:00:00 2001 From: wudidapaopao Date: Thu, 5 Nov 2026 15:46:48 +0000 Subject: [PATCH 3/7] Move the engine pin from v26.11.0-rc.1 to v26.11.0-rc.2 chdb-core v26.11.0-rc.2 is rc.1 plus chdb-core#312, on the same ClickHouse baseline (26.11.1.1). For this binding the visible change is at exit: chdb_shutdown() no longer refuses once a MergeTree table was used, and it now also stops jemalloc's background threads. chdb.h differs from rc.1 only in CHDB_VERSION, so the addon needs no change. This is the same edit the engine release check makes: CHDB_ENGINE_PIN, and LIBCHDB_NPM_VERSION plus the four @chdb/lib-* optionalDependencies at 26.11.0-rc.2.1, the scheme's first packaging of this engine. The prose that names the pin moves with it: the README's durable section, the test workflow header and the design doc's still-to-do item. The update_libchdb.sh comment names this build's chdb_version() string, and rc.1 joins the engines that an object written on rc.2 refuses. --- .github/workflows/chdb-node-test.yml | 2 +- README.md | 2 +- docs/design/durable-control-plane.md | 2 +- package.json | 8 ++++---- update_libchdb.sh | 10 +++++----- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/.github/workflows/chdb-node-test.yml b/.github/workflows/chdb-node-test.yml index a5f43e9..81bab5c 100644 --- a/.github/workflows/chdb-node-test.yml +++ b/.github/workflows/chdb-node-test.yml @@ -10,7 +10,7 @@ on: # update_libchdb.sh pins a published chdb-core libchdb across all four target # platforms, so the full v2 + v3 suite is expected green here. The pin is -# currently the v26.11.0-rc.1 release candidate. v26.7.3 was the first stable +# currently the v26.11.0-rc.2 release candidate. v26.7.3 was the first stable # release to export the Durable V1 ABI (chdb_backup_database_n / # chdb_restore_database_n / chdb_classify_query_n); v26.7.2-rc.2 was the # candidate that introduced them. diff --git a/README.md b/README.md index d36c66d..95f8e04 100644 --- a/README.md +++ b/README.md @@ -200,7 +200,7 @@ and the sync policy with `@clickhouse/client`. > is injected; Node callers get a default engine over this package's own addon > from `chdb/durable/node`. > -> Requires an engine exporting the durable ABI — currently `26.11.0-rc.1`. +> Requires an engine exporting the durable ABI — currently `26.11.0-rc.2`. > Compatibility is a floor rather than an equality: an object records > `min_reader` and `backup_format`, and any engine at or above that floor opens > it. An object written by `26.7.2-rc.2` stays readable on `26.7.3` and later. diff --git a/docs/design/durable-control-plane.md b/docs/design/durable-control-plane.md index 4a3775c..e4efdc0 100644 --- a/docs/design/durable-control-plane.md +++ b/docs/design/durable-control-plane.md @@ -724,7 +724,7 @@ this package: 1. Publish `@chdb/lib-` packages built from a tree that binds the durable ABI. `optionalDependencies` and `update_libchdb.sh` already name - `26.11.0-rc.1.1`, and the release workflow derives the subpackage version + `26.11.0-rc.2.1`, and the release workflow derives the subpackage version from the latter — but nothing has been tagged since, so that version is not on npm at all. The newest published is `26.7.0-stable.1` (main `chdb@3.3.0` pins it), which carries an addon from before this work and exports none of diff --git a/package.json b/package.json index 67c32bf..125808d 100644 --- a/package.json +++ b/package.json @@ -121,10 +121,10 @@ "node-gyp-build": "^4.6.0" }, "optionalDependencies": { - "@chdb/lib-darwin-arm64": "26.11.0-rc.1.1", - "@chdb/lib-darwin-x64": "26.11.0-rc.1.1", - "@chdb/lib-linux-arm64-gnu": "26.11.0-rc.1.1", - "@chdb/lib-linux-x64-gnu": "26.11.0-rc.1.1" + "@chdb/lib-darwin-arm64": "26.11.0-rc.2.1", + "@chdb/lib-darwin-x64": "26.11.0-rc.2.1", + "@chdb/lib-linux-arm64-gnu": "26.11.0-rc.2.1", + "@chdb/lib-linux-x64-gnu": "26.11.0-rc.2.1" }, "peerDependencies": { "@clickhouse/client": ">=1.23.0", diff --git a/update_libchdb.sh b/update_libchdb.sh index 0bc3c8a..8f491ec 100755 --- a/update_libchdb.sh +++ b/update_libchdb.sh @@ -17,15 +17,15 @@ set -e # without them. v26.7.3 is the first stable release that exports them. # # The pin is currently a release candidate, and chdb_version() on it returns the -# full "26.11.0-rc.1", suffix and all. Durable records that string in +# full "26.11.0-rc.2", suffix and all. Durable records that string in # head.engine.version and raises min_reader to it on every write. A pre-release # sorts below the release it leads to, so an object written against this RC -# stays readable by the eventual stable, but every earlier engine (26.9.x and -# before) is refused it with engine_incompatible from then on. +# stays readable by the eventual stable, but every earlier engine (26.11.0-rc.1, +# 26.9.x and before) is refused it with engine_incompatible from then on. # # Keep the pin on its own line and literal: the release check greps for it when # it proposes a bump. -CHDB_ENGINE_PIN=v26.11.0-rc.1 +CHDB_ENGINE_PIN=v26.11.0-rc.2 # CHDB_ENGINE_VERSION overrides the pin, which is how the release check runs the # suite against an engine this repository has not adopted yet. It is a different @@ -53,7 +53,7 @@ LATEST_RELEASE="${CHDB_ENGINE_VERSION:-$CHDB_ENGINE_PIN}" # ERR_DLOPEN_FAILED instead. # # The publish and cleanroom workflows read CHDB_LIB_VERSION from here. -LIBCHDB_NPM_VERSION=26.11.0-rc.1.1 +LIBCHDB_NPM_VERSION=26.11.0-rc.2.1 # Download the correct version based on the platform case "$(uname -s)" in From 505b9e382580b4c5fa356e6f1510bfb554186240 Mon Sep 17 00:00:00 2001 From: wudidapaopao Date: Thu, 5 Nov 2026 15:46:48 +0000 Subject: [PATCH 4/7] Check that a process exits silently after using MergeTree On v26.11.0-rc.1 every Node process that had used a MergeTree table printed "chDB global pool shutdown refused: ..." and "chDB shutdown failed: global thread pool did not become idle" at exit: chdb_shutdown() refused while the MergeTree background executors were still alive. The exit code stayed 0, and the existing exit test uses no MergeTree, so nothing here noticed. rc.2 fixes it in the engine (chdb-core#312). The new case leaves a MergeTree session open for the exit hook to close and requires an empty stderr. Against rc.1 it fails with exactly those two lines; against rc.2 it passes. --- test/v3/engine-exit.test.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/v3/engine-exit.test.ts b/test/v3/engine-exit.test.ts index 5eca9a4..543e22b 100644 --- a/test/v3/engine-exit.test.ts +++ b/test/v3/engine-exit.test.ts @@ -21,4 +21,28 @@ describe('engine process teardown (chdb-node#97)', () => { expect(child.stdout.trim()).toBe('42') expect(child.stderr).toBe('') }, 20_000) + + // Up to chdb-core v26.11.0-rc.1, chdb_shutdown() refused once a MergeTree + // table had been used: the background executors outlived the last + // connection, and rc.1 printed "chDB global pool shutdown refused" and + // "chDB shutdown failed" on every such exit. rc.2 destroys them on shutdown + // (chdb-core#312). The session stays open, so the exit hook closes it. + it('exits silently after using a MergeTree table', () => { + const entry = join(process.cwd(), 'index.js') + const script = ` + const { Session } = require(${JSON.stringify(entry)}); + const s = new Session(); + s.query('CREATE TABLE t (id UInt64) ENGINE = MergeTree ORDER BY id'); + s.query('INSERT INTO t SELECT number FROM numbers(1000)'); + process.stdout.write(s.query('SELECT count() FROM t', 'CSV')); + ` + const child = spawnSync(process.execPath, ['-e', script], { + encoding: 'utf8', + timeout: 15_000, + }) + expect(child.error).toBeUndefined() + expect(child.status).toBe(0) + expect(child.stdout.trim()).toBe('1000') + expect(child.stderr).toBe('') + }, 20_000) }) From 87583308b0098b8b7d0feddf43f57f3e338035c7 Mon Sep 17 00:00:00 2001 From: wudidapaopao Date: Thu, 5 Nov 2026 16:55:42 +0000 Subject: [PATCH 5/7] Close the default connection and stop the engine on process exit A process.exit() with MergeTree merges in flight on the standalone query() connection crashed in exit() (SIGSEGV, or "mutex lock failed: Invalid argument"), every time on v26.11.0-rc.2, after chDB's crash handler waited about 30 s. The 'exit' hook closed only Sessions, and Node runs no environment cleanup hooks on process.exit(), so the default connection was first closed by the addon's atexit backstop. That handler is registered at the first connect, and atexit handlers run in reverse order of registration: by then every static constructed later, including the ones queries and merges construct, had been destroyed, and the server teardown ran onto them. The 'exit' hook is now installed when index.js loads, and after closing the sessions it calls the new native ExitTeardown, which closes every connection no async call is running on, then calls chdb_shutdown(). This runs before exit(), with every static still alive. Only the main thread calls it: the connections and the engine are process-wide, so a worker that exits must not close them under the main thread. Deno.exit() skips the 'exit' event and dispatches 'unload', so the hook listens for that too under Deno. A connection with a call still running is left open, since closing it under the call aborts the engine and Node joins that libuv thread only after the 'exit' event. For that case the backstop is registered again, so exit() runs it first: after the join and before the destroyed statics. The backstop stays as the fallback. Its comment said it ran before the engine's own exit handlers. Since rc.2 the engine registers its teardown on every start, so after a restart that teardown runs first, and the backstop also runs after every static constructed after the first connect. The env cleanup hook's comment no longer calls it the most reliable cleanup, since it does not run on process.exit(). An 'exit' listener that runs after this hook now finds the engine closed, as it already found its sessions. Two tests in engine-exit.test.ts: process.exit() with merges in flight on the default connection, and with an insert still running on it. Both fail without this change, the first by crashing, the second by crashing or hanging until killed. --- index.js | 26 ++++++++++- lib/chdb_node.cpp | 90 +++++++++++++++++++++++++++++++------ test/v3/engine-exit.test.ts | 68 ++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+), 16 deletions(-) diff --git a/index.js b/index.js index ef94836..950d896 100644 --- a/index.js +++ b/index.js @@ -6,6 +6,7 @@ const chdbNode = require('./dist/loader.js').loadNative(); const { mkdtempSync, rmSync, realpathSync } = fs; const { join, resolve: resolvePath } = require('path'); const os = require('os'); +const { isMainThread } = require('worker_threads'); const { ChdbConnectionError, ChdbClosedError, @@ -568,12 +569,33 @@ let exitHookInstalled = false; function ensureExitHook() { if (exitHookInstalled) return; exitHookInstalled = true; - process.on('exit', () => { + const teardown = () => { for (const s of openSessions) { try { s.close(); } catch (_) { /* best effort */ } } - }); + // Then what the sessions do not cover: the standalone default connection, + // which the addon opens on first use, and any connection close() had to + // defer. The addon closes the idle ones and stops the engine here, before + // C exit(), because its atexit backstop runs after statics the engine + // still needs are destroyed: a process.exit() with MergeTree merges in + // flight on the default connection crashed there. As with the sessions, an + // 'exit' listener that runs after this one finds the engine closed. Main + // thread only: the connections and the engine are process-wide, and a + // worker that exits must not close them under the main thread. + if (isMainThread && typeof chdbNode.ExitTeardown === 'function') { + try { chdbNode.ExitTeardown(); } catch (_) { /* best effort */ } + } + }; + process.on('exit', teardown); + // Deno.exit() skips the process 'exit' event and dispatches 'unload' + // instead. Running both is harmless: the second finds nothing left to do. + if (typeof Deno !== 'undefined' && typeof globalThis.addEventListener === 'function') { + globalThis.addEventListener('unload', teardown); + } } +// Installed up front rather than with the first Session: the standalone +// functions reach the engine through that default connection without one. +ensureExitHook(); // Session class with connection-based path handling const SESSION_SIGNALS = ['SIGINT', 'SIGTERM']; diff --git a/lib/chdb_node.cpp b/lib/chdb_node.cpp index 858de56..026a437 100644 --- a/lib/chdb_node.cpp +++ b/lib/chdb_node.cpp @@ -201,8 +201,9 @@ class ConnGuard { static void erase_arrow_tables_for_conn(chdb_connection *conn); static void clear_all_arrow_tables(); -// Close every live connection. atexit backstop (the JS layer closes sessions on -// 'exit' too); also used so a leaked connection never outlives the process. +// Close every live connection. atexit backstop (the JS layer closes sessions and +// calls ExitTeardownWrapper on 'exit' first); also used so a leaked connection +// never outlives the process. static void hard_close_all() { std::lock_guard lk(g_reg_mu); clear_all_arrow_tables(); @@ -214,20 +215,33 @@ static void hard_close_all() { g_reg.boundKey.clear(); } -// Node's environment cleanup closes connections, but the engine may still own -// background threads when C++ static destructors run. On macOS with core -// v26.9.0, a CLI that queried once could hang indefinitely inside exit(). -// Run this process-wide backstop before the engine's own exit handlers. +// The atexit backstop: close whatever is still open when the process reaches C +// exit(), then stop the engine, so it owns no background threads while C++ +// static destructors run. On macOS with core v26.9.0, a CLI that queried once +// could hang indefinitely inside exit() without it. +// +// It is only the fallback. index.js calls ExitTeardownWrapper (below) from the +// process 'exit' event, which closes everything it safely can before exit(). +// atexit handlers and static destructors run in reverse order of registration, +// and this one is registered once, at the first connect, so it runs after two +// things registered later: the exit teardown the engine registers on every +// start (since chdb-core v26.11.0-rc.2), so after an engine restart that runs +// first, and every function-local static constructed after the first connect, +// including ones a query or a MergeTree merge constructed. Closing a connection +// here tears the server down onto those destroyed statics; with merges in +// flight, a process.exit() on the default connection crashed in this handler. static void shutdown_on_process_exit() { hard_close_all(); (void)chdb_shutdown(); } +// Set at the first connect. Guarded by g_reg_mu: both callers hold it. +static bool g_atexit_registered = false; + static void ensure_atexit() { - static bool done = false; - if (!done) { + if (!g_atexit_registered) { std::atexit(shutdown_on_process_exit); - done = true; + g_atexit_registered = true; } } @@ -253,8 +267,9 @@ static chdb_connection *get_default_conn(char **error_message) { g_reg.defaultConn = c; g_reg.boundKey = ""; g_reg.conns.insert(c); - // Register only after chdb_connect initialized the engine: atexit handlers - // run in reverse registration order, so shutdown must precede its teardown. + // Register only after chdb_connect started the engine, so the backstop runs + // before the exit teardown that start registered (see shutdown_on_process_exit + // for what it still runs after). ensure_atexit(); return c; } @@ -1760,6 +1775,49 @@ Napi::Value EngineVersionWrapper(const Napi::CallbackInfo &info) { return Napi::String::New(env, version); } +// Exit teardown, called by index.js from the main thread's process 'exit' +// event: the last point before Node calls C exit(), where every static is still +// alive (shutdown_on_process_exit explains why the atexit backstop is too late). +// index.js has closed its sessions by then. This closes what is left, the +// standalone default connection above all, and then stops the engine, so +// nothing of chDB runs into Node's own teardown. +// +// A connection an async call is still running on (the in-flight count) stays +// open: closing it under the call aborts the engine, and Node joins that libuv +// thread only after this returns. For those, the backstop is registered once +// more. exit() then runs it first: after Node has joined the thread, and before +// the statics that the first registration runs after. +Napi::Value ExitTeardownWrapper(const Napi::CallbackInfo &info) { + bool started; + bool busy = false; + { + std::lock_guard lk(g_reg_mu); + started = g_atexit_registered; + for (auto it = g_reg.conns.begin(); it != g_reg.conns.end();) { + chdb_connection *c = *it; + if (c && inflight_locked(*c) > 0) { + busy = true; + ++it; + continue; + } + if (c) { + erase_arrow_tables_for_conn(c); + chdb_close_conn(c); + } + if (c == g_reg.defaultConn) g_reg.defaultConn = nullptr; + it = g_reg.conns.erase(it); + } + if (g_reg.conns.empty()) g_reg.boundKey.clear(); + } + if (started) { + if (busy) + std::atexit(shutdown_on_process_exit); + else + (void)chdb_shutdown(); + } + return info.Env().Undefined(); +} + Napi::Object Init(Napi::Env env, Napi::Object exports) { // Export the functions exports.Set("Query", Napi::Function::New(env, QueryWrapper)); @@ -1796,9 +1854,13 @@ Napi::Object Init(Napi::Env env, Napi::Object exports) { exports.Set("ArrowRegisterColumns", Napi::Function::New(env, ArrowRegisterColumnsWrapper)); exports.Set("ArrowUnregister", Napi::Function::New(env, ArrowUnregisterWrapper)); - // Most-reliable exit cleanup: close every live connection on env teardown, - // ahead of (and complementary to) the std::atexit backstop. Both call the - // idempotent hard_close_all, so running twice is harmless. + // For index.js's process 'exit' hook. + exports.Set("ExitTeardown", Napi::Function::New(env, ExitTeardownWrapper)); + + // Close every live connection on env teardown, ahead of the std::atexit + // backstop. Both call the idempotent hard_close_all, so running twice is + // harmless. Node runs no cleanup hooks on process.exit(), which is one reason + // index.js closes on the 'exit' event instead (ExitTeardownWrapper). env.AddCleanupHook([] { hard_close_all(); }); return exports; diff --git a/test/v3/engine-exit.test.ts b/test/v3/engine-exit.test.ts index 543e22b..1c65e8f 100644 --- a/test/v3/engine-exit.test.ts +++ b/test/v3/engine-exit.test.ts @@ -45,4 +45,72 @@ describe('engine process teardown (chdb-node#97)', () => { expect(child.stdout.trim()).toBe('1000') expect(child.stderr).toBe('') }, 20_000) + + // The standalone functions' default connection had no 'exit' hook, so a + // process.exit() left it to the addon's atexit backstop, inside C exit(), + // after statics the server teardown still uses were destroyed. With merges + // in flight that crashed: SIGSEGV, or "mutex lock failed: Invalid argument", + // each after chDB's crash handler waited about 30 s. + it('exits cleanly from process.exit() with merges in flight on the default connection', () => { + const entry = join(process.cwd(), 'index.js') + const script = ` + const { query } = require(${JSON.stringify(entry)}); + query('CREATE TABLE t (id UInt64, s String) ENGINE = MergeTree ORDER BY id'); + let merging = 0; + for (let i = 0; i < 200 && merging === 0; i++) { + query('INSERT INTO t SELECT number, toString(number) FROM numbers(' + i * 20000 + ', 20000)'); + merging = Number(query('SELECT count() FROM system.merges', 'CSV').trim()); + } + process.stdout.write(String(merging)); + process.exit(0); + ` + const child = spawnSync(process.execPath, ['-e', script], { + encoding: 'utf8', + timeout: 20_000, + killSignal: 'SIGKILL', + }) + expect(child.error, child.stderr).toBeUndefined() + expect(child.signal, child.stderr).toBeNull() + expect(child.status, child.stderr).toBe(0) + // Only meaningful if a merge was running when the exit began. + expect(Number(child.stdout)).toBeGreaterThan(0) + expect(child.stderr).toBe('') + }, 30_000) + + // The same with a query still running on the default connection, which the + // 'exit' hook must not close under it. It is closed in exit() instead, by + // the backstop registered again from the hook so that it runs before the + // destroyed statics. Without that this crashed or hung in exit(). + it('exits cleanly from process.exit() with an insert still running on the default connection', () => { + const entry = join(process.cwd(), 'index.js') + const script = ` + const { query, queryAsync } = require(${JSON.stringify(entry)}); + query('CREATE TABLE t (id UInt64, s String) ENGINE = MergeTree ORDER BY id'); + let merging = 0; + for (let i = 0; i < 200 && merging === 0; i++) { + query('INSERT INTO t SELECT number, toString(number) FROM numbers(' + i * 20000 + ', 20000)'); + merging = Number(query('SELECT count() FROM system.merges', 'CSV').trim()); + } + let done = false; + queryAsync('INSERT INTO t SELECT number, toString(number) FROM numbers(10000000)', { format: 'CSV' }) + .then(() => { done = true; }, () => { done = true; }); + setTimeout(() => { + process.stdout.write(merging + ' ' + (done ? 'done' : 'running')); + process.exit(0); + }, 20); + ` + const child = spawnSync(process.execPath, ['-e', script], { + encoding: 'utf8', + timeout: 60_000, + killSignal: 'SIGKILL', + }) + expect(child.error, child.stderr).toBeUndefined() + expect(child.signal, child.stderr).toBeNull() + expect(child.status, child.stderr).toBe(0) + // Only meaningful if merges ran and the insert had not finished at exit. + const [merging, insert] = child.stdout.split(' ') + expect(Number(merging)).toBeGreaterThan(0) + expect(insert).toBe('running') + expect(child.stderr).toBe('') + }, 70_000) }) From d69fda7425bc3382a349d6965c40f2b1e8125e0e Mon Sep 17 00:00:00 2001 From: wudidapaopao Date: Thu, 5 Nov 2026 17:04:42 +0000 Subject: [PATCH 6/7] Call chdb_shutdown() once between the exit paths Every macOS job failed at process exit after the previous commit, with "libc++abi: terminating due to uncaught exception of type std::__1::system_error: mutex lock failed: Invalid argument" and an abort. chdb_shutdown() keeps a function-local static mutex that its first call constructs. ExitTeardown now makes that first call from the 'exit' event, after the atexit backstop was registered, so the mutex's destructor runs before the backstop, and the backstop's own chdb_shutdown() then locked a destroyed mutex. glibc lets that through, which is why Linux passed; macOS returns EINVAL and std::mutex throws. The busy branch hit the same thing the other way round: the backstop registered again from the hook calls it first, and the original backstop called it a second time. The exit paths now share a flag and call chdb_shutdown() once between them. gdb on Linux shows a single call at exit in both cases, where there were two before. --- lib/chdb_node.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/lib/chdb_node.cpp b/lib/chdb_node.cpp index 026a437..cc11d52 100644 --- a/lib/chdb_node.cpp +++ b/lib/chdb_node.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -230,9 +231,21 @@ static void hard_close_all() { // including ones a query or a MergeTree merge constructed. Closing a connection // here tears the server down onto those destroyed statics; with merges in // flight, a process.exit() on the default connection crashed in this handler. +// +// chdb_shutdown() is one of them: it keeps a function-local static mutex, +// constructed by its first call. Once ExitTeardownWrapper has called it, a +// second call from here locks that mutex after its destructor ran, which macOS +// reports as "mutex lock failed: Invalid argument" and aborts. So the exit +// paths call it once between them. +static std::atomic g_exit_shutdown_called{false}; + +static void shutdown_engine_once() { + if (!g_exit_shutdown_called.exchange(true)) (void)chdb_shutdown(); +} + static void shutdown_on_process_exit() { hard_close_all(); - (void)chdb_shutdown(); + shutdown_engine_once(); } // Set at the first connect. Guarded by g_reg_mu: both callers hold it. @@ -1813,7 +1826,7 @@ Napi::Value ExitTeardownWrapper(const Napi::CallbackInfo &info) { if (busy) std::atexit(shutdown_on_process_exit); else - (void)chdb_shutdown(); + shutdown_engine_once(); } return info.Env().Undefined(); } From e4947ee8cf4bd401bf77413ad90004f90219812f Mon Sep 17 00:00:00 2001 From: wudidapaopao Date: Thu, 5 Nov 2026 17:56:55 +0000 Subject: [PATCH 7/7] Close nothing at exit while a worker has the addon loaded Review found a regression in the exit teardown. ExitTeardown, called from the main thread's 'exit' event, closed every connection whose in-flight count was 0. Synchronous calls are not in that count, and Node stops worker threads only after the 'exit' event. A worker looping `new Session()` and `s.query()` while the main thread called process.exit(0) therefore ran its next query on a freed connection: SIGSEGV in chdb_query_n from QueryWithConnection, 3 times out of 3. The addon on main was clean, because only the atexit backstop closed connections, after Node had stopped the workers. The addon now counts the environments that loaded it: Init counts one in and the env cleanup hook counts it out. While another one is alive, ExitTeardown closes nothing and registers the backstop again, as it already does for a connection with an async call running. By the time exit() runs that backstop, Node has stopped and joined every worker. New engine-exit.test.ts case: a worker runs synchronous Session queries in a loop, and the main thread calls process.exit(0). It fails on the previous commit and passes with this one, as does the reviewer's repro for both a worker Session and the default connection. --- lib/chdb_node.cpp | 25 +++++++++++++++++++++---- test/v3/engine-exit.test.ts | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/lib/chdb_node.cpp b/lib/chdb_node.cpp index cc11d52..35c49b4 100644 --- a/lib/chdb_node.cpp +++ b/lib/chdb_node.cpp @@ -251,6 +251,11 @@ static void shutdown_on_process_exit() { // Set at the first connect. Guarded by g_reg_mu: both callers hold it. static bool g_atexit_registered = false; +// Environments that loaded the addon and have not been torn down: the main +// thread, and each worker_threads Worker that required it. Init counts one in, +// its env cleanup hook counts it out. ExitTeardownWrapper reads it. +static std::atomic g_live_envs{0}; + static void ensure_atexit() { if (!g_atexit_registered) { std::atexit(shutdown_on_process_exit); @@ -1800,13 +1805,20 @@ Napi::Value EngineVersionWrapper(const Napi::CallbackInfo &info) { // thread only after this returns. For those, the backstop is registered once // more. exit() then runs it first: after Node has joined the thread, and before // the statics that the first registration runs after. +// +// While another environment has the addon loaded (a worker_threads Worker), +// nothing is closed: that thread may be using any connection, its own sessions +// or the shared default, and synchronous calls are not in the in-flight count. +// Node stops it only after this returns, so closing one here freed it under +// the worker's next query. The backstop is registered again instead; by the +// time exit() runs it, Node has stopped and joined every worker. Napi::Value ExitTeardownWrapper(const Napi::CallbackInfo &info) { bool started; - bool busy = false; + bool busy = g_live_envs.load() > 1; { std::lock_guard lk(g_reg_mu); started = g_atexit_registered; - for (auto it = g_reg.conns.begin(); it != g_reg.conns.end();) { + for (auto it = g_reg.conns.begin(); !busy && it != g_reg.conns.end();) { chdb_connection *c = *it; if (c && inflight_locked(*c) > 0) { busy = true; @@ -1873,8 +1885,13 @@ Napi::Object Init(Napi::Env env, Napi::Object exports) { // Close every live connection on env teardown, ahead of the std::atexit // backstop. Both call the idempotent hard_close_all, so running twice is // harmless. Node runs no cleanup hooks on process.exit(), which is one reason - // index.js closes on the 'exit' event instead (ExitTeardownWrapper). - env.AddCleanupHook([] { hard_close_all(); }); + // index.js closes on the 'exit' event instead (ExitTeardownWrapper). The + // hook also counts this environment out of g_live_envs. + g_live_envs.fetch_add(1); + env.AddCleanupHook([] { + g_live_envs.fetch_sub(1); + hard_close_all(); + }); return exports; } diff --git a/test/v3/engine-exit.test.ts b/test/v3/engine-exit.test.ts index 1c65e8f..f3f0cd4 100644 --- a/test/v3/engine-exit.test.ts +++ b/test/v3/engine-exit.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it } from 'vitest' import { spawnSync } from 'node:child_process' +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' import { join } from 'node:path' describe('engine process teardown (chdb-node#97)', () => { @@ -113,4 +115,38 @@ describe('engine process teardown (chdb-node#97)', () => { expect(insert).toBe('running') expect(child.stderr).toBe('') }, 70_000) + + // A worker_threads Worker holds connections of its own, and Node stops it + // only after the main thread's 'exit' event. Closing them from that event + // freed the worker's Session under its next synchronous query (SIGSEGV). + it('exits cleanly from process.exit() while a worker runs synchronous Session queries', () => { + const entry = join(process.cwd(), 'index.js') + const dir = mkdtempSync(join(tmpdir(), 'chdb-exit-worker-')) + try { + const workerCode = ` + const { Session } = require(${JSON.stringify(entry)}); + const s = new Session(${JSON.stringify(join(dir, 'data'))}); + s.query('SELECT 1'); + require('node:worker_threads').parentPort.postMessage('started'); + for (;;) s.query('SELECT sum(number) FROM numbers(100000000)'); + ` + const script = ` + const { Worker } = require('node:worker_threads'); + require(${JSON.stringify(entry)}); + const w = new Worker(${JSON.stringify(workerCode)}, { eval: true }); + w.on('message', () => setTimeout(() => process.exit(0), 100)); + ` + const child = spawnSync(process.execPath, ['-e', script], { + encoding: 'utf8', + timeout: 20_000, + killSignal: 'SIGKILL', + }) + expect(child.error, child.stderr).toBeUndefined() + expect(child.signal, child.stderr).toBeNull() + expect(child.status, child.stderr).toBe(0) + expect(child.stderr).toBe('') + } finally { + rmSync(dir, { recursive: true, force: true }) + } + }, 30_000) })