From 680c8ad8b7acd5d81dbabf930f3dae621c3901e5 Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Wed, 13 May 2026 21:21:17 +0800 Subject: [PATCH 1/3] fix: serialize moduleInit/cleanupHook across N-API envs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `exportModule` atomically incremented `env_refcount`, but the user's `options.init` and the subsequent `registerDecls` / `options.register` calls were not serialized across environments. When two N-API environments (e.g. Node.js Worker threads loading the same addon at roughly the same time) call into `moduleInit` concurrently, the "second" env's `fetchAdd` returns a non-zero `prev_refcount` and skips the user's init, but it then proceeds to `registerDecls` and exposes the module's exports to JS while the "first" env is still inside the user's `init` hook. JS code on the second env can observe partially-initialized shared state. Guard `moduleInit`'s lifecycle path and `cleanupHook` with a per-module spinlock (same pattern already used in `class_runtime.zig`, introduced in #20). The critical section stays small and uncontended in the common case (single env) — one CAS on attach and one store on detach. --- src/js/export_module.zig | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/js/export_module.zig b/src/js/export_module.zig index aa70ba1..6daceb9 100644 --- a/src/js/export_module.zig +++ b/src/js/export_module.zig @@ -31,8 +31,10 @@ const class_runtime = @import("class_runtime.zig"); /// have been processed and *before* the module's `init` hook (if present). /// `exports` is the JavaScript object that will hold the module's exports. /// -/// The DSL internally manages an atomic refcount for module instances across -/// different N-API environments. +/// The DSL manages an atomic refcount across N-API environments and +/// serializes `init`/`cleanup` so concurrent attaches (e.g. Worker threads +/// loading the same addon) cannot expose exports while init is still +/// running on another thread. /// /// Usage Examples: /// ```zig @@ -64,6 +66,7 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { const State = struct { var env_refcount: std.atomic.Value(u32) = std.atomic.Value(u32).init(0); + var locked: std.atomic.Value(bool) = std.atomic.Value(bool).init(false); // addEnvCleanupHook requires a non-null *Data pointer. const CleanupData = struct { @@ -71,7 +74,20 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { }; var cleanup_data: CleanupData = .{}; + fn lock() void { + while (locked.cmpxchgWeak(false, true, .acquire, .monotonic) != null) { + std.atomic.spinLoopHint(); + } + } + + fn unlock() void { + locked.store(false, .release); + } + fn cleanupHook(_: *CleanupData) void { + lock(); + defer unlock(); + const prev = env_refcount.fetchSub(1, .acq_rel); const new_refcount = prev - 1; if (has_cleanup) { @@ -86,6 +102,9 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { defer context.restoreEnv(prev); if (has_lifecycle) { + State.lock(); + defer State.unlock(); + const prev_refcount = State.env_refcount.fetchAdd(1, .monotonic); var cleanup_hook_registered = false; errdefer if (!cleanup_hook_registered) { From 02b7ad00bcaf8c17feb4c481c93b1744d35ab01e Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Tue, 14 Jul 2026 14:48:00 -0400 Subject: [PATCH 2/3] refactor(js): use blocking lifecycle mutex --- src/js/export_module.zig | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/src/js/export_module.zig b/src/js/export_module.zig index 49d286c..0af1558 100644 --- a/src/js/export_module.zig +++ b/src/js/export_module.zig @@ -67,7 +67,7 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { const State = struct { var env_refcount: std.atomic.Value(u32) = std.atomic.Value(u32).init(0); - var locked: std.atomic.Value(bool) = std.atomic.Value(bool).init(false); + var lifecycle_mutex: std.Io.Mutex = .init; // addEnvCleanupHook requires a non-null *Data pointer. const CleanupData = struct { @@ -75,20 +75,10 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { }; var cleanup_data: CleanupData = .{}; - fn lock() void { - while (locked.cmpxchgWeak(false, true, .acquire, .monotonic) != null) { - std.atomic.spinLoopHint(); - } - } - - fn unlock() void { - locked.store(false, .release); - } - fn cleanupHook(_: *CleanupData) void { if (has_lifecycle) { - lock(); - defer unlock(); + std.Io.Threaded.mutexLock(&lifecycle_mutex); + defer std.Io.Threaded.mutexUnlock(&lifecycle_mutex); const prev = env_refcount.fetchSub(1, .acq_rel); const new_refcount = prev - 1; @@ -106,9 +96,9 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { defer context.restoreEnv(prev); if (has_lifecycle) { - State.lock(); + std.Io.Threaded.mutexLock(&State.lifecycle_mutex); } - defer if (has_lifecycle) State.unlock(); + defer if (has_lifecycle) std.Io.Threaded.mutexUnlock(&State.lifecycle_mutex); io_context.retain(); var cleanup_hook_registered = false; From 92d74cd9b577dc5517e5d76a1850806b3dc3bc2a Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Tue, 14 Jul 2026 14:57:28 -0400 Subject: [PATCH 3/3] fix(js): use Io interface for lifecycle mutex --- src/js/export_module.zig | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/js/export_module.zig b/src/js/export_module.zig index 0af1558..0a4b4a2 100644 --- a/src/js/export_module.zig +++ b/src/js/export_module.zig @@ -77,8 +77,9 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { fn cleanupHook(_: *CleanupData) void { if (has_lifecycle) { - std.Io.Threaded.mutexLock(&lifecycle_mutex); - defer std.Io.Threaded.mutexUnlock(&lifecycle_mutex); + const io = io_context.io(); + lifecycle_mutex.lockUncancelable(io); + defer lifecycle_mutex.unlock(io); const prev = env_refcount.fetchSub(1, .acq_rel); const new_refcount = prev - 1; @@ -95,17 +96,18 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { const prev = context.setEnv(env); defer context.restoreEnv(prev); - if (has_lifecycle) { - std.Io.Threaded.mutexLock(&State.lifecycle_mutex); - } - defer if (has_lifecycle) std.Io.Threaded.mutexUnlock(&State.lifecycle_mutex); - io_context.retain(); var cleanup_hook_registered = false; errdefer if (!cleanup_hook_registered) { io_context.release(); }; + const io = io_context.io(); + if (has_lifecycle) { + State.lifecycle_mutex.lockUncancelable(io); + } + defer if (has_lifecycle) State.lifecycle_mutex.unlock(io); + var prev_refcount: u32 = 0; if (has_lifecycle) { prev_refcount = State.env_refcount.fetchAdd(1, .monotonic);