From 1f746299a707c9dc5b72a178705ea357adbe1263 Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Fri, 10 Apr 2026 17:46:01 +0800 Subject: [PATCH 1/2] fix: improve atomic ordering in ThreadPool and NAPI init - ThreadPool: relax err_flag.load from .acquire to .monotonic in worker hot loops (L154, L260). The flag is a pure early-exit signal with no data dependency on the setter's other writes. Matches the pattern already used in pubkey_cache.zig. - NAPI root: guard shared state initialization with init_mutex to prevent a race where concurrent register calls (e.g. from Node.js Worker threads) could observe partially-initialized state. Add errdefer to roll back env_refcount on init failure so the next caller retries. --- bindings/napi/root.zig | 20 +++++++++++++++----- src/bls/ThreadPool.zig | 4 ++-- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/bindings/napi/root.zig b/bindings/napi/root.zig index 2b38817dc..a4de8f506 100644 --- a/bindings/napi/root.zig +++ b/bindings/napi/root.zig @@ -18,6 +18,10 @@ comptime { /// and torn down only when the last environment exits. var env_refcount: std.atomic.Value(u32) = std.atomic.Value(u32).init(0); +/// Guards shared state initialization so that concurrent `register` calls +/// (e.g. from Node.js Worker threads) cannot observe partially-initialized state. +var init_mutex: std.Thread.Mutex = .{}; + const EnvCleanup = struct { fn hook(_: *EnvCleanup) void { if (env_refcount.fetchSub(1, .acq_rel) == 1) { @@ -33,11 +37,17 @@ const EnvCleanup = struct { var env_cleanup: EnvCleanup = .{}; fn register(env: napi.Env, exports: napi.Value) !void { - if (env_refcount.fetchAdd(1, .monotonic) == 0) { - // First environment — initialize shared state. - try pool.state.init(); - try pubkeys.state.init(); - config.state.init(); + { + init_mutex.lock(); + defer init_mutex.unlock(); + if (env_refcount.fetchAdd(1, .monotonic) == 0) { + // First environment — initialize shared state. + // On failure, roll back the refcount so the next caller retries. + errdefer _ = env_refcount.fetchSub(1, .monotonic); + try pool.state.init(); + try pubkeys.state.init(); + config.state.init(); + } } try env.addEnvCleanupHook(EnvCleanup, &env_cleanup, EnvCleanup.hook); diff --git a/src/bls/ThreadPool.zig b/src/bls/ThreadPool.zig index 4c112b390..3bce464c9 100644 --- a/src/bls/ThreadPool.zig +++ b/src/bls/ThreadPool.zig @@ -151,7 +151,7 @@ fn execVerifyMulti(pool: *ThreadPool, job: *VerifyMultiJob, worker_index: usize) while (true) { const i = job.counter.fetchAdd(1, .monotonic); if (i >= n_elems) break; - if (job.err_flag.load(.acquire)) break; + if (job.err_flag.load(.monotonic)) break; did_work = true; @@ -257,7 +257,7 @@ fn execAggVerify(pool: *ThreadPool, job: *AggVerifyJob, worker_index: usize) voi while (true) { const i = job.counter.fetchAdd(1, .monotonic); if (i >= job.n_elems) break; - if (job.err_flag.load(.acquire)) break; + if (job.err_flag.load(.monotonic)) break; did_work = true; From 6860e0e04b866ad6797299fc268a03202ec27246 Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Fri, 10 Apr 2026 18:28:12 +0800 Subject: [PATCH 2/2] fix: guard NAPI cleanup hook with init_mutex and assert errdefer invariant - Take init_mutex in EnvCleanup.hook to prevent init/deinit race when a Worker thread tears down while another is registering. - Assert env_refcount == 1 in errdefer to enforce the invariant that only our increment is present when init fails. --- bindings/napi/root.zig | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/bindings/napi/root.zig b/bindings/napi/root.zig index a4de8f506..5e655f76a 100644 --- a/bindings/napi/root.zig +++ b/bindings/napi/root.zig @@ -24,6 +24,8 @@ var init_mutex: std.Thread.Mutex = .{}; const EnvCleanup = struct { fn hook(_: *EnvCleanup) void { + init_mutex.lock(); + defer init_mutex.unlock(); if (env_refcount.fetchSub(1, .acq_rel) == 1) { // Last environment — tear down shared state. config.state.deinit(); @@ -43,7 +45,10 @@ fn register(env: napi.Env, exports: napi.Value) !void { if (env_refcount.fetchAdd(1, .monotonic) == 0) { // First environment — initialize shared state. // On failure, roll back the refcount so the next caller retries. - errdefer _ = env_refcount.fetchSub(1, .monotonic); + errdefer { + const old = env_refcount.fetchSub(1, .monotonic); + std.debug.assert(old == 1); + } try pool.state.init(); try pubkeys.state.init(); config.state.init();