From 123382a85ea0e4ca50cfd9768c0b5d99b327e3cd Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Wed, 22 Jul 2026 10:19:11 +0500 Subject: [PATCH 01/10] fix(napi): bound BigInt word reads and i128 conversion napi sets the out word_count to the required count, which can exceed the caller's buffer; slicing by it read out of bounds for BigInts wider than the buffer (panic in safe builds, UB in ReleaseFast). getValueBigintWords returns error.Overflow instead. toI128 now range-checks the magnitude and handles -2^127, which previously tripped @intCast before negation. Co-Authored-By: Claude Fable 5 --- examples/js_dsl/mod.test.ts | 22 ++++++++++++++++++++++ examples/js_dsl/mod.zig | 13 ++++++++++--- src/Value.zig | 7 ++++++- src/js/bigint.zig | 11 +++++++---- 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/examples/js_dsl/mod.test.ts b/examples/js_dsl/mod.test.ts index 42db759..67e4666 100644 --- a/examples/js_dsl/mod.test.ts +++ b/examples/js_dsl/mod.test.ts @@ -110,6 +110,28 @@ describe("primitive types", () => { expect(mod.bigIntSign(-0xffffffffffffffffn)).toEqual(1); }); + it("rejects BigInts wider than the provided word buffer", () => { + expect(() => mod.bigIntFirstWord(2n ** 64n)).toThrow(); + expect(() => mod.bigIntFirstWord(2n ** 200n)).toThrow(); + }); + }); + + describe("toI128", () => { + it("round-trips values across the i128 range", () => { + expect(mod.bigIntToI128String(0n)).toEqual("0"); + expect(mod.bigIntToI128String(123n)).toEqual("123"); + expect(mod.bigIntToI128String(-123n)).toEqual("-123"); + expect(mod.bigIntToI128String(2n ** 127n - 1n)).toEqual((2n ** 127n - 1n).toString()); + expect(mod.bigIntToI128String(-(2n ** 127n))).toEqual((-(2n ** 127n)).toString()); + }); + + it("rejects BigInts outside the i128 range instead of crashing", () => { + expect(() => mod.bigIntToI128String(2n ** 127n)).toThrow(); + expect(() => mod.bigIntToI128String(-(2n ** 127n) - 1n)).toThrow(); + expect(() => mod.bigIntToI128String(2n ** 128n)).toThrow(); + expect(() => mod.bigIntToI128String(2n ** 200n)).toThrow(); + expect(() => mod.bigIntToI128String(-(2n ** 200n))).toThrow(); + }); }); it("tomorrow adds one day", () => { diff --git a/examples/js_dsl/mod.zig b/examples/js_dsl/mod.zig index 99f5438..bc2674f 100644 --- a/examples/js_dsl/mod.zig +++ b/examples/js_dsl/mod.zig @@ -102,11 +102,10 @@ pub fn doubleBigInt(n: BigInt) !BigInt { /// Read a BigInt's first u64 word via `getValueBigintWords` passing `null` for `sign_bit`. /// -/// Throws if word_count > 1. +/// Throws `Overflow` if the BigInt needs more than one word. pub fn bigIntFirstWord(n: BigInt) !Number { var words: [1]u64 = .{0}; - const got = try n.toValue().getValueBigintWords(null, &words); - if (got.len > 1) return error.BigIntTooLarge; + _ = try n.toValue().getValueBigintWords(null, &words); return Number.from(words[0]); } @@ -118,6 +117,14 @@ pub fn bigIntSign(n: BigInt) !Number { return Number.from(@as(u32, sign)); } +/// Convert a BigInt to an i128 and render it as a decimal string. +pub fn bigIntToI128String(n: BigInt) !String { + const v = try n.toI128(); + var buf: [48]u8 = undefined; + const s = std.fmt.bufPrint(&buf, "{d}", .{v}) catch return error.FormatError; + return String.from(s); +} + /// Add one day (86400000ms) to a Date. pub fn tomorrow(d: Date) Date { const ts = d.assertTimestamp(); diff --git a/src/Value.zig b/src/Value.zig index bd3b206..b1a3165 100644 --- a/src/Value.zig +++ b/src/Value.zig @@ -223,16 +223,21 @@ pub fn getValueBigintUint64(self: Value, lossless: ?*bool) NapiError!u64 { /// In Ethereum's context, this is useful for big integers defined in the spec to be /// unsigned. /// +/// Returns `error.Overflow` if the BigInt needs more words than `words` can hold: +/// napi sets the out `word_count` to the *required* count, which may exceed the +/// buffer, so slicing by it unchecked would read out of bounds. +/// /// NOTE: napi's C entry takes `int*` (4-byte aligned). Casting a u1 to int* is UB, since that /// is 4-bytes aligned. We use a local `c_int` for the napi call and narrow back to `u1` for the caller. /// /// Source: https://nodejs.org/api/n-api.html#napi_get_value_bigint_words -pub fn getValueBigintWords(self: Value, sign_bit: ?*u1, words: []u64) NapiError![]u64 { +pub fn getValueBigintWords(self: Value, sign_bit: ?*u1, words: []u64) (NapiError || error{Overflow})![]u64 { var word_count: usize = words.len; var raw_sign: c_int = 0; try status.check( c.napi_get_value_bigint_words(self.env, self.value, &raw_sign, &word_count, words.ptr), ); + if (word_count > words.len) return error.Overflow; // napi guarantees raw_sign ∈ {0, 1} if (sign_bit) |s| s.* = @intCast(raw_sign); return words[0..word_count]; diff --git a/src/js/bigint.zig b/src/js/bigint.zig index fd38ebd..f27d0fd 100644 --- a/src/js/bigint.zig +++ b/src/js/bigint.zig @@ -38,19 +38,22 @@ pub const BigInt = struct { /// /// This function reads the BigInt as two 64-bit words and reconstructs it /// into a Zig `i128`. It handles both positive and negative BigInts. - /// Returns an error if N-API operations fail. + /// Returns `error.Overflow` if the value is outside the i128 range, and an + /// error if N-API operations fail. pub fn toI128(self: BigInt) !i128 { var sign_bit: u1 = 0; var words: [2]u64 = .{ 0, 0 }; const result = try self.val.getValueBigintWords(&sign_bit, &words); - const lo: u128 = result[0]; + const lo: u128 = if (result.len > 0) result[0] else 0; const hi: u128 = if (result.len > 1) result[1] else 0; const magnitude: u128 = (hi << 64) | lo; + const max_positive: u128 = std.math.maxInt(i128); if (sign_bit == 1) { - // Negative: negate the magnitude - if (magnitude == 0) return 0; + if (magnitude > max_positive + 1) return error.Overflow; + if (magnitude == max_positive + 1) return std.math.minInt(i128); return -@as(i128, @intCast(magnitude)); } + if (magnitude > max_positive) return error.Overflow; return @intCast(magnitude); } From e118b09b38c9af806557d9d045ae1574dd49b9ed Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Wed, 22 Jul 2026 10:21:04 +0500 Subject: [PATCH 02/10] fix(napi): tolerate unknown napi enum values Status, ValueType, and TypedarrayType were exhaustive enums populated straight from napi out-params; a status or array type this binding doesn't know (e.g. Float16Array, statuses added in newer Node) was invalid-enum UB in ReleaseFast and a panic in safe builds. Make them non-exhaustive and map unknown values to errors. Co-Authored-By: Claude Fable 5 --- src/Value.zig | 3 ++- src/js/wrap_function.zig | 1 + src/status.zig | 9 +++++++++ src/value_types.zig | 21 ++++++++++++++++++++- 4 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/Value.zig b/src/Value.zig index b1a3165..3f179c9 100644 --- a/src/Value.zig +++ b/src/Value.zig @@ -149,7 +149,8 @@ pub fn getTypedarrayInfo(self: Value) NapiError!TypedarrayInfo { try status.check( c.napi_get_typedarray_info(self.env, self.value, @ptrCast(&info.array_type), &info.length, @ptrCast(&data), @ptrCast(&info.arraybuffer), &info.byte_offset), ); - info.data = data[0 .. info.length * info.array_type.elementSize()]; + const elem_size = info.array_type.elementSize() orelse return error.GenericFailure; + info.data = data[0 .. info.length * elem_size]; return info; } diff --git a/src/js/wrap_function.zig b/src/js/wrap_function.zig index fcaf689..cafabfc 100644 --- a/src/js/wrap_function.zig +++ b/src/js/wrap_function.zig @@ -75,6 +75,7 @@ fn typedArrayName(comptime array_type: napi.value_types.TypedarrayType) []const .float64 => "Float64Array", .bigint64 => "BigInt64Array", .biguint64 => "BigUint64Array", + _ => "TypedArray", }; } diff --git a/src/status.zig b/src/status.zig index c20fc2c..58a016a 100644 --- a/src/status.zig +++ b/src/status.zig @@ -26,6 +26,8 @@ pub const Status = enum(c.napi_status) { would_deadlock = c.napi_would_deadlock, no_external_buffers_allowed = c.napi_no_external_buffers_allowed, cannot_run_js = c.napi_cannot_run_js, + // Non-exhaustive: future Node versions may add statuses. + _, }; pub const NapiError = error{ @@ -80,6 +82,7 @@ pub fn check(code: c_uint) NapiError!void { .would_deadlock => return error.WouldDeadlock, .no_external_buffers_allowed => return error.NoExternalBuffersAllowed, .cannot_run_js => return error.CannotRunJS, + _ => return error.GenericFailure, } } @@ -112,3 +115,9 @@ test "isNapiError identifies napi errors" { try std.testing.expect(isNapiError(error.GenericFailure)); try std.testing.expect(!isNapiError(error.OutOfMemory)); } + +test "check maps unknown status codes to GenericFailure" { + // Future Node versions may return statuses this enum doesn't know about; + // they must map to an error, not invalid-enum UB. + try std.testing.expectError(error.GenericFailure, check(9999)); +} diff --git a/src/value_types.zig b/src/value_types.zig index bd21a00..62485ca 100644 --- a/src/value_types.zig +++ b/src/value_types.zig @@ -34,6 +34,8 @@ pub const ValueType = enum(c.napi_valuetype) { function = c.napi_function, external = c.napi_external, bigint = c.napi_bigint, + // Non-exhaustive: future Node versions may add value types. + _, }; /// https://nodejs.org/api/n-api.html#napi_typedarray_type @@ -49,13 +51,18 @@ pub const TypedarrayType = enum(c.napi_typedarray_type) { float64 = c.napi_float64_array, bigint64 = c.napi_bigint64_array, biguint64 = c.napi_biguint64_array, + // Non-exhaustive: engines already ship array types napi doesn't name yet + // (e.g. Float16Array); unknown values must be representable, not UB. + _, - pub fn elementSize(self: TypedarrayType) usize { + /// Returns null for typedarray types this binding doesn't know about. + pub fn elementSize(self: TypedarrayType) ?usize { switch (self) { .int8, .uint8, .uint8_clamped => return 1, .int16, .uint16 => return 2, .int32, .uint32, .float32 => return 4, .float64, .bigint64, .biguint64 => return 8, + _ => return null, } } @@ -72,10 +79,22 @@ pub const TypedarrayType = enum(c.napi_typedarray_type) { .float64 => return f64, .bigint64 => return i64, .biguint64 => return u64, + _ => @compileError("unknown typedarray type"), } } }; +test "elementSize returns null for unknown typedarray types" { + // e.g. Float16Array on newer engines: unknown values must not be UB. + const unknown: TypedarrayType = @enumFromInt(999); + try @import("std").testing.expect(unknown.elementSize() == null); +} + +test "elementSize covers known typedarray types" { + try @import("std").testing.expectEqual(@as(?usize, 1), TypedarrayType.uint8.elementSize()); + try @import("std").testing.expectEqual(@as(?usize, 8), TypedarrayType.biguint64.elementSize()); +} + /// https://nodejs.org/api/n-api.html#napi_property_attributes pub const PropertyAttributes = enum(c.napi_property_attributes) { default = c.napi_default, From 182d1e2c87986d6bb36375bcb501cfa2955b567c Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Wed, 22 Jul 2026 10:22:24 +0500 Subject: [PATCH 03/10] fix(dsl): reject class constructor calls without new Without new, V8 hands the global proxy as the receiver, so the generated constructor wrapped native state and a finalizer onto globalThis and type-tagged it. Co-Authored-By: Claude Fable 5 --- examples/js_dsl/mod.test.ts | 6 ++++++ src/js/wrap_class.zig | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/examples/js_dsl/mod.test.ts b/examples/js_dsl/mod.test.ts index 67e4666..f2769a6 100644 --- a/examples/js_dsl/mod.test.ts +++ b/examples/js_dsl/mod.test.ts @@ -242,6 +242,12 @@ describe("Counter class", () => { expect(() => getCount.call(new mod.Buffer(4))).toThrow(); }); + it("rejects constructor calls without new", () => { + expect(() => mod.Counter(5)).toThrow(TypeError); + expect(() => mod.Point()).toThrow(TypeError); + expect(() => Reflect.apply(mod.Counter, undefined, [5])).toThrow(TypeError); + }); + it("increments", () => { const c = new mod.Counter(0); c.increment(); diff --git a/src/js/wrap_class.zig b/src/js/wrap_class.zig index bb20974..a48fd85 100644 --- a/src/js/wrap_class.zig +++ b/src/js/wrap_class.zig @@ -360,6 +360,17 @@ pub fn wrapClass(comptime T: type) type { return null; }; + // Without `new`, `this` is the global proxy; wrapping it would + // attach native state and a finalizer to globalThis. + const new_target = e.getNewTarget(cb_info) catch { + e.throwError("", "Failed to get new.target in constructor") catch {}; + return null; + }; + if (new_target.value == null) { + e.throwTypeError("", "Class constructor cannot be invoked without 'new'") catch {}; + return null; + } + // Fast path: materializeClassInstance is creating this instance. // Skip normal init; materialize will wrap the returned JS instance // with the real native pointer after napi_new_instance returns. From 5af75851c5a23dd96209dce1eafd64467d257d15 Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Wed, 22 Jul 2026 10:23:31 +0500 Subject: [PATCH 04/10] fix: memory bugs on napi error paths - wrapTaggedObject destroyed the native object on tagging failure while materializeClassInstance's errdefer destroyed it again (double free); ownership now stays with the caller on error - registerClass linked the entry before addEnvCleanupHook, so a hook failure freed the list head while still reachable (use-after-free) and leaked the ctor ref - module registration aborted via catch unreachable when throwError failed with an exception already pending None of these paths are reachable from the JS test harness (they need napi calls failing mid-sequence), hence no regression tests. Co-Authored-By: Claude Fable 5 --- src/js/class_runtime.zig | 11 +++++++---- src/js/wrap_class.zig | 1 + src/module.zig | 4 +++- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/js/class_runtime.zig b/src/js/class_runtime.zig index edf52ef..e0c468a 100644 --- a/src/js/class_runtime.zig +++ b/src/js/class_runtime.zig @@ -8,12 +8,12 @@ pub fn typeTag(comptime T: type) napi.c.napi_type_tag { }; } +/// On error the caller keeps ownership of `native_object`: the wrap is +/// detached so the finalizer cannot fire, but the object is not destroyed. pub fn wrapTaggedObject(comptime T: type, env: napi.Env, object: napi.Value, native_object: *T) !void { const tag = typeTag(T); try env.wrap(object, T, native_object, defaultFinalize(T), null, null); - errdefer if (env.removeWrap(T, object)) |removed| { - destroyNativeObject(T, removed); - } else |_| {}; + errdefer _ = env.removeWrap(T, object) catch {}; if (!(try env.checkObjectTypeTag(object, tag))) { try env.typeTagObject(object, tag); } @@ -68,9 +68,12 @@ pub fn registerClass(comptime T: type, env: napi.Env, ctor: napi.Value) !void { .ctor_ref = try env.createReference(ctor, 1), .next = State.head, }; - State.head = entry; + errdefer entry.ctor_ref.delete() catch {}; + // Link only after the cleanup hook is registered: a hook failure must not + // leave a freed entry reachable from the list head. try env.addEnvCleanupHook(State.Entry, entry, State.cleanupHook); + State.head = entry; } /// Per-thread marker set by `materializeClassInstance` to tell the generated diff --git a/src/js/wrap_class.zig b/src/js/wrap_class.zig index a48fd85..c8499f0 100644 --- a/src/js/wrap_class.zig +++ b/src/js/wrap_class.zig @@ -411,6 +411,7 @@ pub fn wrapClass(comptime T: type) type { const this_val = napi.Value{ .env = raw_env, .value = this_arg }; class_runtime.wrapTaggedObject(T, e, this_val, obj_ptr) catch { + class_runtime.destroyNativeObject(T, obj_ptr); e.throwError("", "Failed to wrap native object") catch {}; return null; }; diff --git a/src/module.zig b/src/module.zig index d834556..d64ae9a 100644 --- a/src/module.zig +++ b/src/module.zig @@ -13,7 +13,9 @@ pub fn register(comptime f: fn (Env, Value) anyerror!void) void { .value = module, }; f(e, v) catch |err| { - e.throwError(@errorName(err), "Error in module registration") catch unreachable; + // throwError fails if an exception is already pending; that + // exception is the failure report, so never abort here. + e.throwError(@errorName(err), "Error in module registration") catch {}; }; return module; } From 1deb61cde54b0cda66cab30bb2dbf506112b4321 Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Wed, 22 Jul 2026 10:24:39 +0500 Subject: [PATCH 05/10] fix(dsl): implement missing js.Value narrowing helpers expectType and expectTypedArrayOfType were called by every as*() narrowing method but never defined; Zig's lazy analysis hid it until a consumer instantiated one, which then failed to compile. Co-Authored-By: Claude Fable 5 --- examples/js_dsl/mod.test.ts | 19 +++++++++++++++++++ examples/js_dsl/mod.zig | 12 ++++++++++++ src/js/value.zig | 11 +++++++++++ 3 files changed, 42 insertions(+) diff --git a/examples/js_dsl/mod.test.ts b/examples/js_dsl/mod.test.ts index f2769a6..f5f8adf 100644 --- a/examples/js_dsl/mod.test.ts +++ b/examples/js_dsl/mod.test.ts @@ -147,6 +147,25 @@ describe("primitive types", () => { }); }); +describe("value narrowing", () => { + it("narrows numbers", () => { + expect(mod.narrowToNumber(42)).toEqual(42); + }); + + it("rejects non-numbers", () => { + expect(() => mod.narrowToNumber("42")).toThrow(); + expect(() => mod.narrowToNumber({})).toThrow(); + expect(() => mod.narrowToNumber(42n)).toThrow(); + }); + + it("narrows typed arrays by exact subtype", () => { + expect(mod.narrowToUint8ArrayLen(new Uint8Array(3))).toEqual(3); + expect(() => mod.narrowToUint8ArrayLen(new Int8Array(3))).toThrow(); + expect(() => mod.narrowToUint8ArrayLen(new Uint8ClampedArray(3))).toThrow(); + expect(() => mod.narrowToUint8ArrayLen([1, 2, 3])).toThrow(); + }); +}); + // Section 4: Typed Objects describe("typed objects", () => { it("formatConfig returns formatted string", () => { diff --git a/examples/js_dsl/mod.zig b/examples/js_dsl/mod.zig index bc2674f..a0f9754 100644 --- a/examples/js_dsl/mod.zig +++ b/examples/js_dsl/mod.zig @@ -316,6 +316,18 @@ pub const Buffer = struct { // Section 10: Mixed DSL + N-API // ============================================================================ +/// Narrow an untyped value to a Number via the `js.Value` narrowing API. +pub fn narrowToNumber(v: Value) !Number { + return v.asNumber(); +} + +/// Narrow an untyped value to a Uint8Array and return its length. +pub fn narrowToUint8ArrayLen(v: Value) !Number { + const arr = try v.asUint8Array(); + const slice = try arr.toSlice(); + return Number.from(@as(u32, @intCast(slice.len))); +} + /// Return the JS typeof string for any value. /// Demonstrates dropping down to low-level napi to call raw N-API methods. pub fn getTypeOf(val: Value) !String { diff --git a/src/js/value.zig b/src/js/value.zig index 73670ac..84d0aaa 100644 --- a/src/js/value.zig +++ b/src/js/value.zig @@ -245,4 +245,15 @@ pub const Value = struct { pub fn toValue(self: Value) napi.Value { return self.val; } + + fn expectType(self: Value, expected: ValueType) TypeError!void { + const actual = self.val.typeof() catch return error.TypeMismatch; + if (actual != expected) return error.TypeMismatch; + } + + fn expectTypedArrayOfType(self: Value, expected: TypedarrayType) TypeError!void { + if (!(self.val.isTypedarray() catch return error.TypeMismatch)) return error.TypeMismatch; + const info = self.val.getTypedarrayInfo() catch return error.TypeMismatch; + if (info.array_type != expected) return error.TypeMismatch; + } }; From 57fdbf3f44d48a3798448ec74aa9f9515ffd0a30 Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Thu, 23 Jul 2026 12:08:28 +0500 Subject: [PATCH 06/10] docs: note the removeWrap infallibility invariant Co-Authored-By: Claude Fable 5 --- src/js/class_runtime.zig | 1 + 1 file changed, 1 insertion(+) diff --git a/src/js/class_runtime.zig b/src/js/class_runtime.zig index e0c468a..d902f57 100644 --- a/src/js/class_runtime.zig +++ b/src/js/class_runtime.zig @@ -13,6 +13,7 @@ pub fn typeTag(comptime T: type) napi.c.napi_type_tag { pub fn wrapTaggedObject(comptime T: type, env: napi.Env, object: napi.Value, native_object: *T) !void { const tag = typeTag(T); try env.wrap(object, T, native_object, defaultFinalize(T), null, null); + // Assumed infallible: a failing removeWrap leaves the finalizer live and the caller double-frees. errdefer _ = env.removeWrap(T, object) catch {}; if (!(try env.checkObjectTypeTag(object, tag))) { try env.typeTagObject(object, tag); From 4245470a479a64bb7e8d1dc2dfae8be91dcb01aa Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Fri, 24 Jul 2026 14:23:00 +0500 Subject: [PATCH 07/10] docs: explain wrapTaggedObject finalizer ownership Co-Authored-By: Claude Fable 5 --- src/js/class_runtime.zig | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/js/class_runtime.zig b/src/js/class_runtime.zig index d902f57..2159a5a 100644 --- a/src/js/class_runtime.zig +++ b/src/js/class_runtime.zig @@ -8,12 +8,15 @@ pub fn typeTag(comptime T: type) napi.c.napi_type_tag { }; } -/// On error the caller keeps ownership of `native_object`: the wrap is -/// detached so the finalizer cannot fire, but the object is not destroyed. +/// After a successful wrap, N-API owns `native_object` and frees it via the +/// finalizer when the JS object is GC'd. On a later error the caller frees it +/// instead, so the finalizer is detached first — it must not also fire, or the +/// object is freed twice. pub fn wrapTaggedObject(comptime T: type, env: napi.Env, object: napi.Value, native_object: *T) !void { const tag = typeTag(T); try env.wrap(object, T, native_object, defaultFinalize(T), null, null); - // Assumed infallible: a failing removeWrap leaves the finalizer live and the caller double-frees. + // Assumed infallible right after a successful wrap; if removeWrap ever + // failed the finalizer would stay live and both it and the caller would free. errdefer _ = env.removeWrap(T, object) catch {}; if (!(try env.checkObjectTypeTag(object, tag))) { try env.typeTagObject(object, tag); From 6a6231f2d6b0eaaa07e427b3157881050b9b7113 Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Fri, 24 Jul 2026 14:27:42 +0500 Subject: [PATCH 08/10] refactor(example): rename bigIntFirstWord to bigIntSingleWord The helper reads a single-word BigInt and errors otherwise; "first" wrongly implied there are more words to follow. Co-Authored-By: Claude Fable 5 --- examples/js_dsl/mod.test.ts | 10 +++++----- examples/js_dsl/mod.zig | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/examples/js_dsl/mod.test.ts b/examples/js_dsl/mod.test.ts index f5f8adf..036f12a 100644 --- a/examples/js_dsl/mod.test.ts +++ b/examples/js_dsl/mod.test.ts @@ -96,9 +96,9 @@ describe("primitive types", () => { describe("getValueBigintWords", () => { it("reads words with null sign_bit (unsigned only)", () => { - expect(mod.bigIntFirstWord(0n)).toEqual(0); - expect(mod.bigIntFirstWord(1n)).toEqual(1); - expect(mod.bigIntFirstWord(0xdeadbeefn)).toEqual(0xdeadbeef); + expect(mod.bigIntSingleWord(0n)).toEqual(0); + expect(mod.bigIntSingleWord(1n)).toEqual(1); + expect(mod.bigIntSingleWord(0xdeadbeefn)).toEqual(0xdeadbeef); }); it("reads correct sign (non-null sign_bit path)", () => { @@ -111,8 +111,8 @@ describe("primitive types", () => { }); it("rejects BigInts wider than the provided word buffer", () => { - expect(() => mod.bigIntFirstWord(2n ** 64n)).toThrow(); - expect(() => mod.bigIntFirstWord(2n ** 200n)).toThrow(); + expect(() => mod.bigIntSingleWord(2n ** 64n)).toThrow(); + expect(() => mod.bigIntSingleWord(2n ** 200n)).toThrow(); }); }); diff --git a/examples/js_dsl/mod.zig b/examples/js_dsl/mod.zig index a0f9754..38b817f 100644 --- a/examples/js_dsl/mod.zig +++ b/examples/js_dsl/mod.zig @@ -100,10 +100,10 @@ pub fn doubleBigInt(n: BigInt) !BigInt { return BigInt.from(val * 2); } -/// Read a BigInt's first u64 word via `getValueBigintWords` passing `null` for `sign_bit`. +/// Read a single-word BigInt as a u64 via `getValueBigintWords` passing `null` for `sign_bit`. /// /// Throws `Overflow` if the BigInt needs more than one word. -pub fn bigIntFirstWord(n: BigInt) !Number { +pub fn bigIntSingleWord(n: BigInt) !Number { var words: [1]u64 = .{0}; _ = try n.toValue().getValueBigintWords(null, &words); return Number.from(words[0]); From a04a49025eb4117b6d3b5b4dd479e98103a906c5 Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Mon, 27 Jul 2026 10:09:13 +0500 Subject: [PATCH 09/10] =?UTF-8?q?fix:=20address=20review=20=E2=80=94=20reo?= =?UTF-8?q?rder=20wraps,=20fix=20init=20leak=20and=20tagName=20panic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - wrapTaggedObject: tag first, wrap last, so the ownership-transferring wrap is the only fallible step; removes the removeWrap/errdefer and the double-free window entirely - genConstructor: allocate the box before running init so a failed allocation can't strand the resources init_result owns - callback: @tagName panics on a status the (now non-exhaustive) enum doesn't name; use std.enums.tagName with a fallback Co-Authored-By: Claude Fable 5 --- src/callback.zig | 4 +++- src/js/class_runtime.zig | 14 ++++++-------- src/js/wrap_class.zig | 9 +++++++-- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/callback.zig b/src/callback.zig index e5b2453..076772f 100644 --- a/src/callback.zig +++ b/src/callback.zig @@ -40,7 +40,9 @@ pub fn wrapCallback( e.throwError(@errorName(err), "NapiError") catch {}; } else { const message: [:0]const u8 = "Native callback failed"; - e.throwError(@tagName(error_info_status), message) catch {}; + // @tagName would panic on a status the enum doesn't name. + const code = std.enums.tagName(status.Status, error_info_status) orelse "NapiError"; + e.throwError(code, message) catch {}; } } else { e.throwError(@errorName(err), @errorName(err)) catch {}; diff --git a/src/js/class_runtime.zig b/src/js/class_runtime.zig index 2159a5a..216691a 100644 --- a/src/js/class_runtime.zig +++ b/src/js/class_runtime.zig @@ -8,19 +8,17 @@ pub fn typeTag(comptime T: type) napi.c.napi_type_tag { }; } -/// After a successful wrap, N-API owns `native_object` and frees it via the -/// finalizer when the JS object is GC'd. On a later error the caller frees it -/// instead, so the finalizer is detached first — it must not also fire, or the -/// object is freed twice. +/// Tags `object`, then wraps `native_object` into it with a finalizer. +/// +/// The wrap is done last so it is the only fallible step that transfers +/// ownership: once it succeeds N-API's finalizer owns `native_object`, and any +/// earlier failure leaves nothing wrapped, so the caller still owns and frees it. pub fn wrapTaggedObject(comptime T: type, env: napi.Env, object: napi.Value, native_object: *T) !void { const tag = typeTag(T); - try env.wrap(object, T, native_object, defaultFinalize(T), null, null); - // Assumed infallible right after a successful wrap; if removeWrap ever - // failed the finalizer would stay live and both it and the caller would free. - errdefer _ = env.removeWrap(T, object) catch {}; if (!(try env.checkObjectTypeTag(object, tag))) { try env.typeTagObject(object, tag); } + try env.wrap(object, T, native_object, defaultFinalize(T), null, null); } /// Generates a deterministic 64-bit FNV-1a hash at compile-time. diff --git a/src/js/wrap_class.zig b/src/js/wrap_class.zig index c8499f0..0bdf13f 100644 --- a/src/js/wrap_class.zig +++ b/src/js/wrap_class.zig @@ -401,12 +401,17 @@ pub fn wrapClass(comptime T: type) type { }; } - const init_result = callInit(init_fn, args) orelse return null; - + // Allocate before init so a failed allocation can't strand + // the resources init_result owns. const obj_ptr = std.heap.c_allocator.create(T) catch { e.throwError("", "Out of memory allocating native object") catch {}; return null; }; + + const init_result = callInit(init_fn, args) orelse { + std.heap.c_allocator.destroy(obj_ptr); + return null; + }; obj_ptr.* = init_result; const this_val = napi.Value{ .env = raw_env, .value = this_arg }; From ab52f6184c5ee8c10f5fde7f944979d1f568ee79 Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Mon, 27 Jul 2026 10:16:34 +0500 Subject: [PATCH 10/10] refactor(dsl): route native allocations through context.allocator() Funnel the class machinery's create/destroy through the single context.allocator() chokepoint instead of std.heap.c_allocator directly, so the DSL allocator can be swapped in one place. Same allocator today (c_allocator), no behavior change. Co-Authored-By: Claude Fable 5 --- src/js/class_runtime.zig | 11 ++++++----- src/js/wrap_class.zig | 4 ++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/js/class_runtime.zig b/src/js/class_runtime.zig index 216691a..5549f62 100644 --- a/src/js/class_runtime.zig +++ b/src/js/class_runtime.zig @@ -1,5 +1,6 @@ const std = @import("std"); const napi = @import("../napi.zig"); +const context = @import("context.zig"); pub fn typeTag(comptime T: type) napi.c.napi_type_tag { return .{ @@ -43,7 +44,7 @@ pub fn destroyNativeObject(comptime T: type, obj: *T) void { if (@hasDecl(T, "deinit")) { obj.deinit(); } - std.heap.c_allocator.destroy(obj); + context.allocator().destroy(obj); } pub fn defaultFinalize(comptime T: type) napi.FinalizeCallback(T) { @@ -62,8 +63,8 @@ pub fn registerClass(comptime T: type, env: napi.Env, ctor: napi.Value) !void { if (State.find(env.env) != null) return; - const entry = try std.heap.c_allocator.create(State.Entry); - errdefer std.heap.c_allocator.destroy(entry); + const entry = try context.allocator().create(State.Entry); + errdefer context.allocator().destroy(entry); entry.* = .{ .env = env.env, @@ -115,7 +116,7 @@ pub fn consumeMaterialization(comptime T: type, env: napi.Env, this_arg: napi.c. pub fn materializeClassInstance(comptime T: type, env: napi.Env, instance: T, preferred_ctor: ?napi.Value) !napi.Value { const ctor = preferred_ctor orelse try getConstructor(T, env); - const obj_ptr = try std.heap.c_allocator.create(T); + const obj_ptr = try context.allocator().create(T); errdefer destroyNativeObject(T, obj_ptr); obj_ptr.* = instance; @@ -203,7 +204,7 @@ fn state(comptime T: type) type { if (current == entry) { cursor.* = current.next; current.ctor_ref.delete() catch {}; - std.heap.c_allocator.destroy(current); + context.allocator().destroy(current); return; } cursor = ¤t.next; diff --git a/src/js/wrap_class.zig b/src/js/wrap_class.zig index 0bdf13f..4d61239 100644 --- a/src/js/wrap_class.zig +++ b/src/js/wrap_class.zig @@ -403,13 +403,13 @@ pub fn wrapClass(comptime T: type) type { // Allocate before init so a failed allocation can't strand // the resources init_result owns. - const obj_ptr = std.heap.c_allocator.create(T) catch { + const obj_ptr = context.allocator().create(T) catch { e.throwError("", "Out of memory allocating native object") catch {}; return null; }; const init_result = callInit(init_fn, args) orelse { - std.heap.c_allocator.destroy(obj_ptr); + context.allocator().destroy(obj_ptr); return null; }; obj_ptr.* = init_result;