From 00741c5a930a9f787f6376c34bb999ef2f0e0cbc Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Tue, 14 Jul 2026 06:23:02 -0400 Subject: [PATCH 1/6] fix(js): roll back module lifecycle initialization --- src/js/export_module.zig | 62 +++++++++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 10 deletions(-) diff --git a/src/js/export_module.zig b/src/js/export_module.zig index 1b9a3ae..98b2c55 100644 --- a/src/js/export_module.zig +++ b/src/js/export_module.zig @@ -75,11 +75,7 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { fn cleanupHook(_: *CleanupData) void { if (has_lifecycle) { - const prev = env_refcount.fetchSub(1, .acq_rel); - const new_refcount = prev - 1; - if (has_cleanup) { - options.cleanup(new_refcount); - } + releaseLifecycleRef(options, &env_refcount, true); } io_context.release(); } @@ -99,14 +95,16 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { var prev_refcount: u32 = 0; if (has_lifecycle) { prev_refcount = State.env_refcount.fetchAdd(1, .monotonic); - errdefer if (!cleanup_hook_registered) { - _ = State.env_refcount.fetchSub(1, .acq_rel); - }; - if (has_init) { - try options.init(prev_refcount); + options.init(prev_refcount) catch |err| { + releaseLifecycleRef(options, &State.env_refcount, false); + return err; + }; } } + errdefer if (has_lifecycle) { + releaseLifecycleRef(options, &State.env_refcount, has_init); + }; _ = try registerDecls(Module, env, module, 0); @@ -126,6 +124,21 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { napi.module.register(init.moduleInit); } +fn releaseLifecycleRef( + comptime options: anytype, + env_refcount: *std.atomic.Value(u32), + run_cleanup: bool, +) void { + const prev = env_refcount.fetchSub(1, .acq_rel); + std.debug.assert(prev > 0); + const new_refcount = prev - 1; + if (@hasField(@TypeOf(options), "cleanup")) { + if (run_cleanup) { + options.cleanup(new_refcount); + } + } +} + /// Iterates module declarations and registers DSL functions and js_meta classes. fn registerDecls(comptime Module: type, env: napi.Env, module: napi.Value, comptime depth: usize) !bool { const decls = @typeInfo(Module).@"struct".decls; @@ -222,3 +235,32 @@ test "exportModule shares a single js.io across the env lifecycle" { try std.testing.expectEqual(a.userdata, b.userdata); try std.testing.expectEqual(a.vtable, b.vtable); } + +test "exportModule rolls back lifecycle state only after init succeeds" { + // Model failures before and after init returns to verify cleanup is paired only with a + // successfully initialized environment while the refcount is restored in both cases. + const Hooks = struct { + var cleanup_calls: u32 = 0; + var cleanup_refcount: u32 = 0; + + fn cleanup(refcount: u32) void { + cleanup_calls += 1; + cleanup_refcount = refcount; + } + }; + const options = .{ .cleanup = Hooks.cleanup }; + + Hooks.cleanup_calls = 0; + Hooks.cleanup_refcount = 0; + + var env_refcount = std.atomic.Value(u32).init(2); + releaseLifecycleRef(options, &env_refcount, false); + try std.testing.expectEqual(1, env_refcount.load(.monotonic)); + try std.testing.expectEqual(0, Hooks.cleanup_calls); + + env_refcount.store(2, .monotonic); + releaseLifecycleRef(options, &env_refcount, true); + try std.testing.expectEqual(1, env_refcount.load(.monotonic)); + try std.testing.expectEqual(1, Hooks.cleanup_calls); + try std.testing.expectEqual(1, Hooks.cleanup_refcount); +} From d6bc9cf2b2b3bedde6bb5cbfbdd38335448ecf98 Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Tue, 14 Jul 2026 06:52:59 -0400 Subject: [PATCH 2/6] refactor(js): make lifecycle rollback ownership explicit --- src/js/export_module.zig | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/js/export_module.zig b/src/js/export_module.zig index 98b2c55..e8c57f3 100644 --- a/src/js/export_module.zig +++ b/src/js/export_module.zig @@ -75,7 +75,10 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { fn cleanupHook(_: *CleanupData) void { if (has_lifecycle) { - releaseLifecycleRef(options, &env_refcount, true); + releaseLifecycleRef( + &env_refcount, + if (has_cleanup) options.cleanup else null, + ); } io_context.release(); } @@ -93,19 +96,22 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { }; var prev_refcount: u32 = 0; + var init_succeeded = false; if (has_lifecycle) { prev_refcount = State.env_refcount.fetchAdd(1, .monotonic); - if (has_init) { - options.init(prev_refcount) catch |err| { - releaseLifecycleRef(options, &State.env_refcount, false); - return err; - }; - } } - errdefer if (has_lifecycle) { - releaseLifecycleRef(options, &State.env_refcount, has_init); + errdefer if (has_lifecycle and !cleanup_hook_registered) { + releaseLifecycleRef( + &State.env_refcount, + if (has_cleanup and init_succeeded) options.cleanup else null, + ); }; + if (has_init) { + try options.init(prev_refcount); + init_succeeded = true; + } + _ = try registerDecls(Module, env, module, 0); if (has_register) { @@ -118,6 +124,7 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { State.cleanupHook, ); cleanup_hook_registered = true; + errdefer comptime unreachable; } }; @@ -125,17 +132,14 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { } fn releaseLifecycleRef( - comptime options: anytype, env_refcount: *std.atomic.Value(u32), - run_cleanup: bool, + cleanup: ?*const fn (u32) void, ) void { const prev = env_refcount.fetchSub(1, .acq_rel); std.debug.assert(prev > 0); const new_refcount = prev - 1; - if (@hasField(@TypeOf(options), "cleanup")) { - if (run_cleanup) { - options.cleanup(new_refcount); - } + if (cleanup) |callback| { + callback(new_refcount); } } @@ -248,18 +252,16 @@ test "exportModule rolls back lifecycle state only after init succeeds" { cleanup_refcount = refcount; } }; - const options = .{ .cleanup = Hooks.cleanup }; - Hooks.cleanup_calls = 0; Hooks.cleanup_refcount = 0; var env_refcount = std.atomic.Value(u32).init(2); - releaseLifecycleRef(options, &env_refcount, false); + releaseLifecycleRef(&env_refcount, null); try std.testing.expectEqual(1, env_refcount.load(.monotonic)); try std.testing.expectEqual(0, Hooks.cleanup_calls); env_refcount.store(2, .monotonic); - releaseLifecycleRef(options, &env_refcount, true); + releaseLifecycleRef(&env_refcount, Hooks.cleanup); try std.testing.expectEqual(1, env_refcount.load(.monotonic)); try std.testing.expectEqual(1, Hooks.cleanup_calls); try std.testing.expectEqual(1, Hooks.cleanup_refcount); From 8cbef438d4d3f0b73fdaa7fb493f757bd86dfa81 Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Tue, 14 Jul 2026 07:04:17 -0400 Subject: [PATCH 3/6] refactor(js): model module attach lifecycle explicitly --- src/js/export_module.zig | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/src/js/export_module.zig b/src/js/export_module.zig index e8c57f3..f60fc05 100644 --- a/src/js/export_module.zig +++ b/src/js/export_module.zig @@ -63,6 +63,14 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { const has_cleanup = @hasField(@TypeOf(options), "cleanup"); const has_register = @hasField(@TypeOf(options), "register"); const has_lifecycle = has_init or has_cleanup; + const cleanup: ?*const fn (u32) void = if (has_cleanup) options.cleanup else null; + + const AttachState = enum { + io_retained, + lifecycle_retained, + lifecycle_initialized, + cleanup_hook_registered, + }; const State = struct { var env_refcount: std.atomic.Value(u32) = std.atomic.Value(u32).init(0); @@ -75,10 +83,7 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { fn cleanupHook(_: *CleanupData) void { if (has_lifecycle) { - releaseLifecycleRef( - &env_refcount, - if (has_cleanup) options.cleanup else null, - ); + releaseLifecycleRef(&env_refcount, cleanup); } io_context.release(); } @@ -90,26 +95,29 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { defer context.restoreEnv(prev); io_context.retain(); - var cleanup_hook_registered = false; - errdefer if (!cleanup_hook_registered) { - io_context.release(); + var attach_state: AttachState = .io_retained; + errdefer switch (attach_state) { + .io_retained => io_context.release(), + .lifecycle_retained => { + releaseLifecycleRef(&State.env_refcount, null); + io_context.release(); + }, + .lifecycle_initialized => { + releaseLifecycleRef(&State.env_refcount, cleanup); + io_context.release(); + }, + .cleanup_hook_registered => unreachable, }; var prev_refcount: u32 = 0; - var init_succeeded = false; if (has_lifecycle) { prev_refcount = State.env_refcount.fetchAdd(1, .monotonic); + attach_state = .lifecycle_retained; } - errdefer if (has_lifecycle and !cleanup_hook_registered) { - releaseLifecycleRef( - &State.env_refcount, - if (has_cleanup and init_succeeded) options.cleanup else null, - ); - }; if (has_init) { try options.init(prev_refcount); - init_succeeded = true; + attach_state = .lifecycle_initialized; } _ = try registerDecls(Module, env, module, 0); @@ -123,7 +131,7 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { &State.cleanup_data, State.cleanupHook, ); - cleanup_hook_registered = true; + attach_state = .cleanup_hook_registered; errdefer comptime unreachable; } }; From 20c5d30bd81a160614fb0ede31df6b35664b7dbe Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Tue, 14 Jul 2026 07:23:51 -0400 Subject: [PATCH 4/6] refactor(js): simplify module init rollback --- src/js/export_module.zig | 35 +++++++++-------------------------- 1 file changed, 9 insertions(+), 26 deletions(-) diff --git a/src/js/export_module.zig b/src/js/export_module.zig index f60fc05..1dfb22b 100644 --- a/src/js/export_module.zig +++ b/src/js/export_module.zig @@ -64,13 +64,7 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { const has_register = @hasField(@TypeOf(options), "register"); const has_lifecycle = has_init or has_cleanup; const cleanup: ?*const fn (u32) void = if (has_cleanup) options.cleanup else null; - - const AttachState = enum { - io_retained, - lifecycle_retained, - lifecycle_initialized, - cleanup_hook_registered, - }; + const rollback_cleanup_after_init: ?*const fn (u32) void = if (has_init) cleanup else null; const State = struct { var env_refcount: std.atomic.Value(u32) = std.atomic.Value(u32).init(0); @@ -95,30 +89,21 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { defer context.restoreEnv(prev); io_context.retain(); - var attach_state: AttachState = .io_retained; - errdefer switch (attach_state) { - .io_retained => io_context.release(), - .lifecycle_retained => { - releaseLifecycleRef(&State.env_refcount, null); - io_context.release(); - }, - .lifecycle_initialized => { - releaseLifecycleRef(&State.env_refcount, cleanup); - io_context.release(); - }, - .cleanup_hook_registered => unreachable, - }; + errdefer io_context.release(); + var rollback_cleanup: ?*const fn (u32) void = null; var prev_refcount: u32 = 0; if (has_lifecycle) { prev_refcount = State.env_refcount.fetchAdd(1, .monotonic); - attach_state = .lifecycle_retained; } + errdefer if (has_lifecycle) { + releaseLifecycleRef(&State.env_refcount, rollback_cleanup); + }; if (has_init) { try options.init(prev_refcount); - attach_state = .lifecycle_initialized; } + rollback_cleanup = rollback_cleanup_after_init; _ = try registerDecls(Module, env, module, 0); @@ -131,7 +116,6 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { &State.cleanup_data, State.cleanupHook, ); - attach_state = .cleanup_hook_registered; errdefer comptime unreachable; } }; @@ -248,9 +232,8 @@ test "exportModule shares a single js.io across the env lifecycle" { try std.testing.expectEqual(a.vtable, b.vtable); } -test "exportModule rolls back lifecycle state only after init succeeds" { - // Model failures before and after init returns to verify cleanup is paired only with a - // successfully initialized environment while the refcount is restored in both cases. +test "releaseLifecycleRef optionally runs cleanup" { + // The caller arms cleanup only after init succeeds. Refcount restoration is unconditional. const Hooks = struct { var cleanup_calls: u32 = 0; var cleanup_refcount: u32 = 0; From 7f6563a76ca11cc6502650a45c6f78f38c93e50b Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Tue, 14 Jul 2026 13:59:14 -0400 Subject: [PATCH 5/6] refactor(js): minimize lifecycle rollback --- src/js/export_module.zig | 71 ++++++++++++++-------------------------- 1 file changed, 24 insertions(+), 47 deletions(-) diff --git a/src/js/export_module.zig b/src/js/export_module.zig index 1dfb22b..40eb0c7 100644 --- a/src/js/export_module.zig +++ b/src/js/export_module.zig @@ -63,8 +63,6 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { const has_cleanup = @hasField(@TypeOf(options), "cleanup"); const has_register = @hasField(@TypeOf(options), "register"); const has_lifecycle = has_init or has_cleanup; - const cleanup: ?*const fn (u32) void = if (has_cleanup) options.cleanup else null; - const rollback_cleanup_after_init: ?*const fn (u32) void = if (has_init) cleanup else null; const State = struct { var env_refcount: std.atomic.Value(u32) = std.atomic.Value(u32).init(0); @@ -77,7 +75,11 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { fn cleanupHook(_: *CleanupData) void { if (has_lifecycle) { - releaseLifecycleRef(&env_refcount, cleanup); + const prev = env_refcount.fetchSub(1, .acq_rel); + const new_refcount = prev - 1; + if (has_cleanup) { + options.cleanup(new_refcount); + } } io_context.release(); } @@ -89,21 +91,34 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { defer context.restoreEnv(prev); io_context.retain(); - errdefer io_context.release(); + var cleanup_hook_registered = false; + errdefer if (!cleanup_hook_registered) { + io_context.release(); + }; - var rollback_cleanup: ?*const fn (u32) void = null; var prev_refcount: u32 = 0; + var init_succeeded = false; if (has_lifecycle) { prev_refcount = State.env_refcount.fetchAdd(1, .monotonic); } - errdefer if (has_lifecycle) { - releaseLifecycleRef(&State.env_refcount, rollback_cleanup); + errdefer if (!cleanup_hook_registered) { + if (has_lifecycle) { + const prev_refcount_rollback = State.env_refcount.fetchSub(1, .acq_rel); + std.debug.assert(prev_refcount_rollback > 0); + const new_refcount = prev_refcount_rollback - 1; + + if (has_cleanup) { + if (init_succeeded) { + options.cleanup(new_refcount); + } + } + } }; if (has_init) { try options.init(prev_refcount); + init_succeeded = true; } - rollback_cleanup = rollback_cleanup_after_init; _ = try registerDecls(Module, env, module, 0); @@ -116,25 +131,13 @@ pub fn exportModule(comptime Module: type, comptime options: anytype) void { &State.cleanup_data, State.cleanupHook, ); - errdefer comptime unreachable; + cleanup_hook_registered = true; } }; napi.module.register(init.moduleInit); } -fn releaseLifecycleRef( - env_refcount: *std.atomic.Value(u32), - cleanup: ?*const fn (u32) void, -) void { - const prev = env_refcount.fetchSub(1, .acq_rel); - std.debug.assert(prev > 0); - const new_refcount = prev - 1; - if (cleanup) |callback| { - callback(new_refcount); - } -} - /// Iterates module declarations and registers DSL functions and js_meta classes. fn registerDecls(comptime Module: type, env: napi.Env, module: napi.Value, comptime depth: usize) !bool { const decls = @typeInfo(Module).@"struct".decls; @@ -231,29 +234,3 @@ test "exportModule shares a single js.io across the env lifecycle" { try std.testing.expectEqual(a.userdata, b.userdata); try std.testing.expectEqual(a.vtable, b.vtable); } - -test "releaseLifecycleRef optionally runs cleanup" { - // The caller arms cleanup only after init succeeds. Refcount restoration is unconditional. - const Hooks = struct { - var cleanup_calls: u32 = 0; - var cleanup_refcount: u32 = 0; - - fn cleanup(refcount: u32) void { - cleanup_calls += 1; - cleanup_refcount = refcount; - } - }; - Hooks.cleanup_calls = 0; - Hooks.cleanup_refcount = 0; - - var env_refcount = std.atomic.Value(u32).init(2); - releaseLifecycleRef(&env_refcount, null); - try std.testing.expectEqual(1, env_refcount.load(.monotonic)); - try std.testing.expectEqual(0, Hooks.cleanup_calls); - - env_refcount.store(2, .monotonic); - releaseLifecycleRef(&env_refcount, Hooks.cleanup); - try std.testing.expectEqual(1, env_refcount.load(.monotonic)); - try std.testing.expectEqual(1, Hooks.cleanup_calls); - try std.testing.expectEqual(1, Hooks.cleanup_refcount); -} From 1835f96a71ae03353a6c001facb8125ce147b77b Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Thu, 16 Jul 2026 07:39:06 -0400 Subject: [PATCH 6/6] docs: document module lifecycle rollback --- README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/README.md b/README.md index a76ef6e..851b537 100644 --- a/README.md +++ b/README.md @@ -339,6 +339,13 @@ comptime { } ``` +Module initialization is transactional: + +- If `init` fails, the environment refcount is restored and `cleanup` is not called. +- If `init` succeeds but module registration fails, the environment refcount is restored and + `cleanup` is called. +- After successful registration, `cleanup` runs when the environment exits. + This enables safe shared-state initialization for worker thread scenarios. ---