From d97ae6e809a517674b0c81f5ff1838d71e20c724 Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Wed, 29 Jul 2026 14:05:05 +0800 Subject: [PATCH 1/6] fix: preserve external typed array ownership --- examples/js_dsl/mod.test.ts | 2 + src/js/typed_arrays.zig | 94 ++++++++++++++++++++++++++++++------- 2 files changed, 80 insertions(+), 16 deletions(-) diff --git a/examples/js_dsl/mod.test.ts b/examples/js_dsl/mod.test.ts index c694890..1a07340 100644 --- a/examples/js_dsl/mod.test.ts +++ b/examples/js_dsl/mod.test.ts @@ -244,6 +244,8 @@ describe("typed arrays", () => { for (const tc of test_cases) { const result = mod.externalUint8Array(tc.input); expect(result).toBeInstanceOf(Uint8Array); + expect(Buffer.isBuffer(result)).toBe(false); + expect(Array.from(result)).toEqual(tc.input); } }); }); diff --git a/src/js/typed_arrays.zig b/src/js/typed_arrays.zig index 87eb5cd..6cb1a5d 100644 --- a/src/js/typed_arrays.zig +++ b/src/js/typed_arrays.zig @@ -60,30 +60,80 @@ pub fn TypedArray(comptime Element: type, comptime array_type: TypedarrayType) t /// native buffer is freed by a finalizer when V8 collects the ArrayBuffer. pub fn fromExternal(slice: []const Element) !Self { const e = context.env(); - const buf = try context.allocator().dupe(Element, slice); - const byte_len = slice.len * @sizeOf(Element); - const len_hint: ?*anyopaque = @ptrFromInt(slice.len); - const finalize_cb = comptime napi.wrapSliceFinalizeCallback(Element, externalFinalizer); - const arraybuffer = e.createExternalArrayBuffer(std.mem.sliceAsBytes(buf), finalize_cb, len_hint) catch |err| { - context.allocator().free(buf); - return err; + + if (byte_len == 0) { + const arraybuffer = try e.createArrayBuffer(0, null); + const val = try e.createTypedarray(array_type, 0, arraybuffer, 0); + return .{ .val = val }; + } + + const finalizer_context = try createFinalizerContext(context.allocator(), slice); + const arraybuffer = e.createExternalArrayBuffer( + std.mem.sliceAsBytes(finalizer_context.data), + externalFinalizer, + finalizer_context, + ) catch |err| { + if (err != error.NoExternalBuffersAllowed) return err; + defer release(finalizer_context); + + const fallback = try e.createArrayBufferCopy( + std.mem.sliceAsBytes(finalizer_context.data), + null, + ); + const val = try e.createTypedarray(array_type, slice.len, fallback, 0); + return .{ .val = val }; }; _ = try e.adjustExternalMemory(@intCast(byte_len)); + finalizer_context.accounted = true; const val = try e.createTypedarray(array_type, slice.len, arraybuffer, 0); return .{ .val = val }; } - /// Finalizer for buffers allocated by `fromExternal`. Frees the native - /// allocation and reverses the matching `adjustExternalMemory` accounting. - /// - /// Caller is responsible for calling a matching `adjustExternalMemory` at - /// the appropriate callsite to let V8 know about native heap memory usage. - fn externalFinalizer(env: napi.Env, data: []Element) void { - const byte_len = data.len * @sizeOf(Element); - context.allocator().free(data); - _ = env.adjustExternalMemory(-@as(i64, @intCast(byte_len))) catch {}; + const FinalizerContext = struct { + allocator: std.mem.Allocator, + data: []Element, + accounted: bool = false, + }; + + fn createFinalizerContext( + allocator: std.mem.Allocator, + slice: []const Element, + ) !*FinalizerContext { + const data = try allocator.dupe(Element, slice); + errdefer allocator.free(data); + + const finalizer_context = try allocator.create(FinalizerContext); + finalizer_context.* = .{ + .allocator = allocator, + .data = data, + }; + return finalizer_context; + } + + fn externalFinalizer( + env: napi.c.napi_env, + finalize_data: ?*anyopaque, + finalize_hint: ?*anyopaque, + ) callconv(.c) void { + const finalizer_context: *FinalizerContext = + @ptrCast(@alignCast(finalize_hint orelse unreachable)); + std.debug.assert( + finalize_data == @as(?*anyopaque, @ptrCast(finalizer_context.data.ptr)), + ); + if (finalizer_context.accounted) { + const byte_len = finalizer_context.data.len * @sizeOf(Element); + const e = napi.Env{ .env = env }; + _ = e.adjustExternalMemory(-@as(i64, @intCast(byte_len))) catch {}; + } + release(finalizer_context); + } + + fn release(finalizer_context: *FinalizerContext) void { + const allocator = finalizer_context.allocator; + allocator.free(finalizer_context.data); + allocator.destroy(finalizer_context); } /// Creates a new JavaScript TypedArray from a Zig slice by copying the data. @@ -172,3 +222,15 @@ test "TypedArray exposes expected subtype metadata" { try @import("std").testing.expect(Uint8Array.expected_array_type == .uint8); try @import("std").testing.expect(Float64Array.expected_array_type == .float64); } + +test "TypedArray releases data when finalizer context allocation fails" { + var failing_allocator = std.testing.FailingAllocator.init(std.testing.allocator, .{ + .fail_index = 1, + }); + + try std.testing.expectError( + error.OutOfMemory, + Uint8Array.createFinalizerContext(failing_allocator.allocator(), &.{ 1, 2, 3 }), + ); + try std.testing.expectEqual(@as(usize, 1), failing_allocator.deallocations); +} From f33c83b3e21234e2f1122f6999024f1d627c7e6f Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Wed, 29 Jul 2026 19:33:55 +0800 Subject: [PATCH 2/6] fix: preserve external typed array error behavior --- examples/js_dsl/mod.test.ts | 1 - src/js/typed_arrays.zig | 28 ++++++++++------------------ 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/examples/js_dsl/mod.test.ts b/examples/js_dsl/mod.test.ts index 1a07340..086d5ac 100644 --- a/examples/js_dsl/mod.test.ts +++ b/examples/js_dsl/mod.test.ts @@ -244,7 +244,6 @@ describe("typed arrays", () => { for (const tc of test_cases) { const result = mod.externalUint8Array(tc.input); expect(result).toBeInstanceOf(Uint8Array); - expect(Buffer.isBuffer(result)).toBe(false); expect(Array.from(result)).toEqual(tc.input); } }); diff --git a/src/js/typed_arrays.zig b/src/js/typed_arrays.zig index 6cb1a5d..d33563c 100644 --- a/src/js/typed_arrays.zig +++ b/src/js/typed_arrays.zig @@ -62,27 +62,17 @@ pub fn TypedArray(comptime Element: type, comptime array_type: TypedarrayType) t const e = context.env(); const byte_len = slice.len * @sizeOf(Element); - if (byte_len == 0) { - const arraybuffer = try e.createArrayBuffer(0, null); - const val = try e.createTypedarray(array_type, 0, arraybuffer, 0); - return .{ .val = val }; - } - const finalizer_context = try createFinalizerContext(context.allocator(), slice); const arraybuffer = e.createExternalArrayBuffer( std.mem.sliceAsBytes(finalizer_context.data), externalFinalizer, finalizer_context, ) catch |err| { - if (err != error.NoExternalBuffersAllowed) return err; - defer release(finalizer_context); - - const fallback = try e.createArrayBufferCopy( - std.mem.sliceAsBytes(finalizer_context.data), - null, - ); - const val = try e.createTypedarray(array_type, slice.len, fallback, 0); - return .{ .val = val }; + if (err == error.NoExternalBuffersAllowed) { + release(finalizer_context); + } + // Other failures may occur after the finalizer has taken ownership. + return err; }; _ = try e.adjustExternalMemory(@intCast(byte_len)); @@ -119,9 +109,11 @@ pub fn TypedArray(comptime Element: type, comptime array_type: TypedarrayType) t ) callconv(.c) void { const finalizer_context: *FinalizerContext = @ptrCast(@alignCast(finalize_hint orelse unreachable)); - std.debug.assert( - finalize_data == @as(?*anyopaque, @ptrCast(finalizer_context.data.ptr)), - ); + if (finalizer_context.data.len > 0) { + std.debug.assert( + finalize_data == @as(?*anyopaque, @ptrCast(finalizer_context.data.ptr)), + ); + } if (finalizer_context.accounted) { const byte_len = finalizer_context.data.len * @sizeOf(Element); const e = napi.Env{ .env = env }; From ba428573754ede206cb8fc1e06dfc95c61af4c1b Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Wed, 29 Jul 2026 23:29:28 +0800 Subject: [PATCH 3/6] fix: harden external typed array ownership --- examples/js_dsl/mod.test.ts | 46 ++++++++- examples/js_dsl/mod.zig | 6 ++ src/js/typed_arrays.zig | 191 +++++++++++++++++++++++++++++++++--- 3 files changed, 226 insertions(+), 17 deletions(-) diff --git a/examples/js_dsl/mod.test.ts b/examples/js_dsl/mod.test.ts index 086d5ac..3e7f24f 100644 --- a/examples/js_dsl/mod.test.ts +++ b/examples/js_dsl/mod.test.ts @@ -1,8 +1,10 @@ -import { describe, it, expect } from "vitest"; +import { execFileSync } from "node:child_process"; import { createRequire } from "node:module"; +import { describe, expect, it } from "vitest"; const require = createRequire(import.meta.url); -const mod = require("../../zig-out/lib/example_js_dsl.node"); +const addonPath = require.resolve("../../zig-out/lib/example_js_dsl.node"); +const mod = require(addonPath); function expectTypeErrorWithMessage(fn: () => unknown, message: string) { try { @@ -247,6 +249,46 @@ describe("typed arrays", () => { expect(Array.from(result)).toEqual(tc.input); } }); + + it("does not allocate external backing while an exception is pending", () => { + expect(() => mod.externalUint8ArrayWithPendingException()).toThrow( + "pending exception before external allocation", + ); + }); + + it("releases external typed-array memory through its GC finalizer", () => { + const script = ` + const addon = require(${JSON.stringify(addonPath)}); + global.gc(); + + const length = 4 * 1024 * 1024; + const noiseAllowance = 512 * 1024; + const baseline = process.memoryUsage().external; + let value = addon.externalUint8Array(new Array(length).fill(7)); + const retained = process.memoryUsage().external - baseline; + if (retained < length - noiseAllowance) { + throw new Error(\`external memory accounting increased by only \${retained} bytes\`); + } + value = null; + + const deadline = Date.now() + 5000; + function collect() { + global.gc(); + const remaining = process.memoryUsage().external - baseline; + if (remaining <= noiseAllowance) return; + if (Date.now() >= deadline) { + throw new Error(\`external typed array retained \${remaining} bytes after GC\`); + } + setImmediate(collect); + } + setImmediate(collect); + `; + + execFileSync(process.execPath, ["--expose-gc", "-e", script], { + stdio: "pipe", + timeout: 10_000, + }); + }); }); // Section 7: Promises diff --git a/examples/js_dsl/mod.zig b/examples/js_dsl/mod.zig index 0d13d9c..0135741 100644 --- a/examples/js_dsl/mod.zig +++ b/examples/js_dsl/mod.zig @@ -233,6 +233,12 @@ pub fn externalUint8Array(arr: Array) !Uint8Array { return Uint8Array.fromExternal(tmp); } +/// Attempts external allocation after throwing a JavaScript exception. +pub fn externalUint8ArrayWithPendingException() !Uint8Array { + try js.env().throwError("ERR_PENDING_EXCEPTION", "pending exception before external allocation"); + return Uint8Array.fromExternal(&.{ 1, 2, 3 }); +} + // ============================================================================ // Section 7: Promises // ============================================================================ diff --git a/src/js/typed_arrays.zig b/src/js/typed_arrays.zig index d33563c..a80c0b5 100644 --- a/src/js/typed_arrays.zig +++ b/src/js/typed_arrays.zig @@ -60,23 +60,16 @@ pub fn TypedArray(comptime Element: type, comptime array_type: TypedarrayType) t /// native buffer is freed by a finalizer when V8 collects the ArrayBuffer. pub fn fromExternal(slice: []const Element) !Self { const e = context.env(); - const byte_len = slice.len * @sizeOf(Element); + if (try e.isExceptionPending()) return error.PendingException; const finalizer_context = try createFinalizerContext(context.allocator(), slice); - const arraybuffer = e.createExternalArrayBuffer( - std.mem.sliceAsBytes(finalizer_context.data), - externalFinalizer, + const arraybuffer = try transferToExternalArrayBuffer( + e, finalizer_context, - ) catch |err| { - if (err == error.NoExternalBuffersAllowed) { - release(finalizer_context); - } - // Other failures may occur after the finalizer has taken ownership. - return err; - }; + createNapiExternalArrayBuffer, + ); - _ = try e.adjustExternalMemory(@intCast(byte_len)); - finalizer_context.accounted = true; + try accountExternalMemory(e, finalizer_context, napi.Env.adjustExternalMemory); const val = try e.createTypedarray(array_type, slice.len, arraybuffer, 0); return .{ .val = val }; } @@ -85,6 +78,10 @@ pub fn TypedArray(comptime Element: type, comptime array_type: TypedarrayType) t allocator: std.mem.Allocator, data: []Element, accounted: bool = false, + + fn externalMemorySize(self: *const FinalizerContext) usize { + return self.data.len * @sizeOf(Element) + @sizeOf(FinalizerContext); + } }; fn createFinalizerContext( @@ -102,6 +99,46 @@ pub fn TypedArray(comptime Element: type, comptime array_type: TypedarrayType) t return finalizer_context; } + fn transferToExternalArrayBuffer( + e: napi.Env, + finalizer_context: *FinalizerContext, + comptime create_arraybuffer: anytype, + ) napi.status.NapiError!napi.Value { + return create_arraybuffer(e, finalizer_context) catch |err| { + // These statuses are returned before N-API installs the finalizer. + switch (err) { + error.NoExternalBuffersAllowed, + error.CannotRunJS, + => release(finalizer_context), + else => {}, + } + return err; + }; + } + + fn createNapiExternalArrayBuffer( + e: napi.Env, + finalizer_context: *FinalizerContext, + ) napi.status.NapiError!napi.Value { + return e.createExternalArrayBuffer( + std.mem.sliceAsBytes(finalizer_context.data), + externalFinalizer, + finalizer_context, + ); + } + + fn accountExternalMemory( + e: napi.Env, + finalizer_context: *FinalizerContext, + comptime adjust_external_memory: anytype, + ) napi.status.NapiError!void { + _ = try adjust_external_memory( + e, + @intCast(finalizer_context.externalMemorySize()), + ); + finalizer_context.accounted = true; + } + fn externalFinalizer( env: napi.c.napi_env, finalize_data: ?*anyopaque, @@ -115,9 +152,9 @@ pub fn TypedArray(comptime Element: type, comptime array_type: TypedarrayType) t ); } if (finalizer_context.accounted) { - const byte_len = finalizer_context.data.len * @sizeOf(Element); + const accounted_bytes: i64 = @intCast(finalizer_context.externalMemorySize()); const e = napi.Env{ .env = env }; - _ = e.adjustExternalMemory(-@as(i64, @intCast(byte_len))) catch {}; + _ = e.adjustExternalMemory(-accounted_bytes) catch {}; } release(finalizer_context); } @@ -226,3 +263,127 @@ test "TypedArray releases data when finalizer context allocation fails" { ); try std.testing.expectEqual(@as(usize, 1), failing_allocator.deallocations); } + +test "TypedArray release frees data and finalizer context" { + var tracking_allocator = std.testing.FailingAllocator.init(std.testing.allocator, .{}); + + const finalizer_context = try Uint8Array.createFinalizerContext( + tracking_allocator.allocator(), + &.{ 1, 2, 3 }, + ); + const allocations = tracking_allocator.allocations; + Uint8Array.release(finalizer_context); + + try std.testing.expectEqual(@as(usize, 2), allocations); + try std.testing.expectEqual(@as(usize, 2), tracking_allocator.deallocations); + try std.testing.expectEqual(tracking_allocator.allocated_bytes, tracking_allocator.freed_bytes); +} + +test "TypedArray releases local ownership when external buffers are rejected" { + const Reject = struct { + fn noExternalBuffers( + _: napi.Env, + _: *Uint8Array.FinalizerContext, + ) napi.status.NapiError!napi.Value { + return error.NoExternalBuffersAllowed; + } + + fn cannotRunJS( + _: napi.Env, + _: *Uint8Array.FinalizerContext, + ) napi.status.NapiError!napi.Value { + return error.CannotRunJS; + } + }; + + inline for (.{ + .{ error.NoExternalBuffersAllowed, Reject.noExternalBuffers }, + .{ error.CannotRunJS, Reject.cannotRunJS }, + }) |case| { + var tracking_allocator = std.testing.FailingAllocator.init(std.testing.allocator, .{}); + + const finalizer_context = try Uint8Array.createFinalizerContext( + tracking_allocator.allocator(), + &.{ 1, 2, 3 }, + ); + try std.testing.expectError( + case[0], + Uint8Array.transferToExternalArrayBuffer( + .{ .env = null }, + finalizer_context, + case[1], + ), + ); + + try std.testing.expectEqual(@as(usize, 2), tracking_allocator.deallocations); + try std.testing.expectEqual( + tracking_allocator.allocated_bytes, + tracking_allocator.freed_bytes, + ); + } +} + +test "TypedArray preserves finalizer ownership after possible transfer" { + const FailAfterTransfer = struct { + fn create( + _: napi.Env, + finalizer_context: *Uint8Array.FinalizerContext, + ) napi.status.NapiError!napi.Value { + Uint8Array.release(finalizer_context); + return error.GenericFailure; + } + }; + var tracking_allocator = std.testing.FailingAllocator.init(std.testing.allocator, .{}); + + const finalizer_context = try Uint8Array.createFinalizerContext( + tracking_allocator.allocator(), + &.{ 1, 2, 3 }, + ); + try std.testing.expectError( + error.GenericFailure, + Uint8Array.transferToExternalArrayBuffer( + .{ .env = null }, + finalizer_context, + FailAfterTransfer.create, + ), + ); + + try std.testing.expectEqual(@as(usize, 2), tracking_allocator.deallocations); + try std.testing.expectEqual(tracking_allocator.allocated_bytes, tracking_allocator.freed_bytes); +} + +test "TypedArray records external memory only after adjustment succeeds" { + const Adjust = struct { + fn fail(_: napi.Env, _: i64) napi.status.NapiError!i64 { + return error.GenericFailure; + } + + fn succeed(_: napi.Env, bytes: i64) napi.status.NapiError!i64 { + const expected = @sizeOf(Uint8Array.FinalizerContext) + 3; + if (bytes != expected) return error.GenericFailure; + return bytes; + } + }; + const finalizer_context = try Uint8Array.createFinalizerContext( + std.testing.allocator, + &.{ 1, 2, 3 }, + ); + defer Uint8Array.release(finalizer_context); + + try std.testing.expectEqual( + @sizeOf(Uint8Array.FinalizerContext) + 3, + finalizer_context.externalMemorySize(), + ); + try std.testing.expectError( + error.GenericFailure, + Uint8Array.accountExternalMemory(.{ .env = null }, finalizer_context, Adjust.fail), + ); + try std.testing.expect(!finalizer_context.accounted); + + try Uint8Array.accountExternalMemory( + .{ .env = null }, + finalizer_context, + Adjust.succeed, + ); + try std.testing.expect(finalizer_context.accounted); +} From 41808ba5d52becb97e4dbcb23a19796a5202f3c5 Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Thu, 30 Jul 2026 09:06:17 +0800 Subject: [PATCH 4/6] refactor: narrow external typed array ownership fix --- examples/js_dsl/mod.test.ts | 46 +------- examples/js_dsl/mod.zig | 6 -- src/js/typed_arrays.zig | 203 ++++-------------------------------- 3 files changed, 23 insertions(+), 232 deletions(-) diff --git a/examples/js_dsl/mod.test.ts b/examples/js_dsl/mod.test.ts index 3e7f24f..086d5ac 100644 --- a/examples/js_dsl/mod.test.ts +++ b/examples/js_dsl/mod.test.ts @@ -1,10 +1,8 @@ -import { execFileSync } from "node:child_process"; +import { describe, it, expect } from "vitest"; import { createRequire } from "node:module"; -import { describe, expect, it } from "vitest"; const require = createRequire(import.meta.url); -const addonPath = require.resolve("../../zig-out/lib/example_js_dsl.node"); -const mod = require(addonPath); +const mod = require("../../zig-out/lib/example_js_dsl.node"); function expectTypeErrorWithMessage(fn: () => unknown, message: string) { try { @@ -249,46 +247,6 @@ describe("typed arrays", () => { expect(Array.from(result)).toEqual(tc.input); } }); - - it("does not allocate external backing while an exception is pending", () => { - expect(() => mod.externalUint8ArrayWithPendingException()).toThrow( - "pending exception before external allocation", - ); - }); - - it("releases external typed-array memory through its GC finalizer", () => { - const script = ` - const addon = require(${JSON.stringify(addonPath)}); - global.gc(); - - const length = 4 * 1024 * 1024; - const noiseAllowance = 512 * 1024; - const baseline = process.memoryUsage().external; - let value = addon.externalUint8Array(new Array(length).fill(7)); - const retained = process.memoryUsage().external - baseline; - if (retained < length - noiseAllowance) { - throw new Error(\`external memory accounting increased by only \${retained} bytes\`); - } - value = null; - - const deadline = Date.now() + 5000; - function collect() { - global.gc(); - const remaining = process.memoryUsage().external - baseline; - if (remaining <= noiseAllowance) return; - if (Date.now() >= deadline) { - throw new Error(\`external typed array retained \${remaining} bytes after GC\`); - } - setImmediate(collect); - } - setImmediate(collect); - `; - - execFileSync(process.execPath, ["--expose-gc", "-e", script], { - stdio: "pipe", - timeout: 10_000, - }); - }); }); // Section 7: Promises diff --git a/examples/js_dsl/mod.zig b/examples/js_dsl/mod.zig index 0135741..0d13d9c 100644 --- a/examples/js_dsl/mod.zig +++ b/examples/js_dsl/mod.zig @@ -233,12 +233,6 @@ pub fn externalUint8Array(arr: Array) !Uint8Array { return Uint8Array.fromExternal(tmp); } -/// Attempts external allocation after throwing a JavaScript exception. -pub fn externalUint8ArrayWithPendingException() !Uint8Array { - try js.env().throwError("ERR_PENDING_EXCEPTION", "pending exception before external allocation"); - return Uint8Array.fromExternal(&.{ 1, 2, 3 }); -} - // ============================================================================ // Section 7: Promises // ============================================================================ diff --git a/src/js/typed_arrays.zig b/src/js/typed_arrays.zig index a80c0b5..94fc5fc 100644 --- a/src/js/typed_arrays.zig +++ b/src/js/typed_arrays.zig @@ -60,16 +60,28 @@ pub fn TypedArray(comptime Element: type, comptime array_type: TypedarrayType) t /// native buffer is freed by a finalizer when V8 collects the ArrayBuffer. pub fn fromExternal(slice: []const Element) !Self { const e = context.env(); - if (try e.isExceptionPending()) return error.PendingException; + const byte_len = slice.len * @sizeOf(Element); const finalizer_context = try createFinalizerContext(context.allocator(), slice); - const arraybuffer = try transferToExternalArrayBuffer( - e, + const arraybuffer = e.createExternalArrayBuffer( + std.mem.sliceAsBytes(finalizer_context.data), + externalFinalizer, finalizer_context, - createNapiExternalArrayBuffer, - ); + ) catch |err| { + // These statuses are returned before N-API installs the finalizer. + switch (err) { + error.NoExternalBuffersAllowed, + error.PendingException, + error.CannotRunJS, + => release(finalizer_context), + // Other failures may occur after the finalizer has taken ownership. + else => {}, + } + return err; + }; - try accountExternalMemory(e, finalizer_context, napi.Env.adjustExternalMemory); + _ = try e.adjustExternalMemory(@intCast(byte_len)); + finalizer_context.accounted = true; const val = try e.createTypedarray(array_type, slice.len, arraybuffer, 0); return .{ .val = val }; } @@ -78,10 +90,6 @@ pub fn TypedArray(comptime Element: type, comptime array_type: TypedarrayType) t allocator: std.mem.Allocator, data: []Element, accounted: bool = false, - - fn externalMemorySize(self: *const FinalizerContext) usize { - return self.data.len * @sizeOf(Element) + @sizeOf(FinalizerContext); - } }; fn createFinalizerContext( @@ -99,62 +107,17 @@ pub fn TypedArray(comptime Element: type, comptime array_type: TypedarrayType) t return finalizer_context; } - fn transferToExternalArrayBuffer( - e: napi.Env, - finalizer_context: *FinalizerContext, - comptime create_arraybuffer: anytype, - ) napi.status.NapiError!napi.Value { - return create_arraybuffer(e, finalizer_context) catch |err| { - // These statuses are returned before N-API installs the finalizer. - switch (err) { - error.NoExternalBuffersAllowed, - error.CannotRunJS, - => release(finalizer_context), - else => {}, - } - return err; - }; - } - - fn createNapiExternalArrayBuffer( - e: napi.Env, - finalizer_context: *FinalizerContext, - ) napi.status.NapiError!napi.Value { - return e.createExternalArrayBuffer( - std.mem.sliceAsBytes(finalizer_context.data), - externalFinalizer, - finalizer_context, - ); - } - - fn accountExternalMemory( - e: napi.Env, - finalizer_context: *FinalizerContext, - comptime adjust_external_memory: anytype, - ) napi.status.NapiError!void { - _ = try adjust_external_memory( - e, - @intCast(finalizer_context.externalMemorySize()), - ); - finalizer_context.accounted = true; - } - fn externalFinalizer( env: napi.c.napi_env, - finalize_data: ?*anyopaque, + _: ?*anyopaque, finalize_hint: ?*anyopaque, ) callconv(.c) void { const finalizer_context: *FinalizerContext = @ptrCast(@alignCast(finalize_hint orelse unreachable)); - if (finalizer_context.data.len > 0) { - std.debug.assert( - finalize_data == @as(?*anyopaque, @ptrCast(finalizer_context.data.ptr)), - ); - } if (finalizer_context.accounted) { - const accounted_bytes: i64 = @intCast(finalizer_context.externalMemorySize()); + const byte_len = finalizer_context.data.len * @sizeOf(Element); const e = napi.Env{ .env = env }; - _ = e.adjustExternalMemory(-accounted_bytes) catch {}; + _ = e.adjustExternalMemory(-@as(i64, @intCast(byte_len))) catch {}; } release(finalizer_context); } @@ -263,127 +226,3 @@ test "TypedArray releases data when finalizer context allocation fails" { ); try std.testing.expectEqual(@as(usize, 1), failing_allocator.deallocations); } - -test "TypedArray release frees data and finalizer context" { - var tracking_allocator = std.testing.FailingAllocator.init(std.testing.allocator, .{}); - - const finalizer_context = try Uint8Array.createFinalizerContext( - tracking_allocator.allocator(), - &.{ 1, 2, 3 }, - ); - const allocations = tracking_allocator.allocations; - Uint8Array.release(finalizer_context); - - try std.testing.expectEqual(@as(usize, 2), allocations); - try std.testing.expectEqual(@as(usize, 2), tracking_allocator.deallocations); - try std.testing.expectEqual(tracking_allocator.allocated_bytes, tracking_allocator.freed_bytes); -} - -test "TypedArray releases local ownership when external buffers are rejected" { - const Reject = struct { - fn noExternalBuffers( - _: napi.Env, - _: *Uint8Array.FinalizerContext, - ) napi.status.NapiError!napi.Value { - return error.NoExternalBuffersAllowed; - } - - fn cannotRunJS( - _: napi.Env, - _: *Uint8Array.FinalizerContext, - ) napi.status.NapiError!napi.Value { - return error.CannotRunJS; - } - }; - - inline for (.{ - .{ error.NoExternalBuffersAllowed, Reject.noExternalBuffers }, - .{ error.CannotRunJS, Reject.cannotRunJS }, - }) |case| { - var tracking_allocator = std.testing.FailingAllocator.init(std.testing.allocator, .{}); - - const finalizer_context = try Uint8Array.createFinalizerContext( - tracking_allocator.allocator(), - &.{ 1, 2, 3 }, - ); - try std.testing.expectError( - case[0], - Uint8Array.transferToExternalArrayBuffer( - .{ .env = null }, - finalizer_context, - case[1], - ), - ); - - try std.testing.expectEqual(@as(usize, 2), tracking_allocator.deallocations); - try std.testing.expectEqual( - tracking_allocator.allocated_bytes, - tracking_allocator.freed_bytes, - ); - } -} - -test "TypedArray preserves finalizer ownership after possible transfer" { - const FailAfterTransfer = struct { - fn create( - _: napi.Env, - finalizer_context: *Uint8Array.FinalizerContext, - ) napi.status.NapiError!napi.Value { - Uint8Array.release(finalizer_context); - return error.GenericFailure; - } - }; - var tracking_allocator = std.testing.FailingAllocator.init(std.testing.allocator, .{}); - - const finalizer_context = try Uint8Array.createFinalizerContext( - tracking_allocator.allocator(), - &.{ 1, 2, 3 }, - ); - try std.testing.expectError( - error.GenericFailure, - Uint8Array.transferToExternalArrayBuffer( - .{ .env = null }, - finalizer_context, - FailAfterTransfer.create, - ), - ); - - try std.testing.expectEqual(@as(usize, 2), tracking_allocator.deallocations); - try std.testing.expectEqual(tracking_allocator.allocated_bytes, tracking_allocator.freed_bytes); -} - -test "TypedArray records external memory only after adjustment succeeds" { - const Adjust = struct { - fn fail(_: napi.Env, _: i64) napi.status.NapiError!i64 { - return error.GenericFailure; - } - - fn succeed(_: napi.Env, bytes: i64) napi.status.NapiError!i64 { - const expected = @sizeOf(Uint8Array.FinalizerContext) + 3; - if (bytes != expected) return error.GenericFailure; - return bytes; - } - }; - const finalizer_context = try Uint8Array.createFinalizerContext( - std.testing.allocator, - &.{ 1, 2, 3 }, - ); - defer Uint8Array.release(finalizer_context); - - try std.testing.expectEqual( - @sizeOf(Uint8Array.FinalizerContext) + 3, - finalizer_context.externalMemorySize(), - ); - try std.testing.expectError( - error.GenericFailure, - Uint8Array.accountExternalMemory(.{ .env = null }, finalizer_context, Adjust.fail), - ); - try std.testing.expect(!finalizer_context.accounted); - - try Uint8Array.accountExternalMemory( - .{ .env = null }, - finalizer_context, - Adjust.succeed, - ); - try std.testing.expect(finalizer_context.accounted); -} From 7b1bc1fa7bc19ee46167bbe75b45476da7fac321 Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Thu, 30 Jul 2026 09:56:10 +0800 Subject: [PATCH 5/6] feat: add owned typed arrays --- README.md | 18 +++ examples/js_dsl/mod.test.ts | 9 +- examples/js_dsl/mod.zig | 15 +++ src/js.zig | 12 ++ src/js/typed_arrays.zig | 224 ++++++++++++++++++++++++++---------- src/js/wrap_function.zig | 11 +- 6 files changed, 226 insertions(+), 63 deletions(-) diff --git a/README.md b/README.md index 09cec47..de5a52f 100644 --- a/README.md +++ b/README.md @@ -91,6 +91,7 @@ c.count; // 1 (getter, not a method call) | `Function` | `Function` | `call(args)` | | `Value` | `any` | `isNumber()`, `asNumber()`, type checking/narrowing | | `Uint8Array` etc. | `TypedArray` | `toSlice()`, `from(slice)` | +| `OwnedUint8Array` etc. | `TypedArray` | `fromOwnedSlice(allocator, data)`, `fromSlice(allocator, data)` | | `Promise(T)` | `Promise` | `resolve(value)`, `reject(err)` | --- @@ -288,6 +289,23 @@ pub fn sum(data: Uint8Array) !Number { } ``` +Use an owned return type to transfer an allocator-owned slice to JavaScript +without copying its elements: + +```zig +pub fn serialize() !js.OwnedUint8Array { + const allocator = js.allocator(); + const data = try allocator.alloc(u8, 32); + // Fill data... + return js.OwnedUint8Array.fromOwnedSlice(allocator, data); +} +``` + +Returning the value consumes it, including on conversion failure. JavaScript +releases the allocation through the ArrayBuffer finalizer, so the allocator must +remain valid until that finalizer runs. If external ArrayBuffers are unsupported, +the original error is returned; no copy fallback is performed. + ### Promises ```zig diff --git a/examples/js_dsl/mod.test.ts b/examples/js_dsl/mod.test.ts index 086d5ac..637457e 100644 --- a/examples/js_dsl/mod.test.ts +++ b/examples/js_dsl/mod.test.ts @@ -244,7 +244,14 @@ describe("typed arrays", () => { for (const tc of test_cases) { const result = mod.externalUint8Array(tc.input); expect(result).toBeInstanceOf(Uint8Array); - expect(Array.from(result)).toEqual(tc.input); + } + }); + + it("ownedUint8Array returns allocator-owned native data", () => { + for (const input of [[], [10, 20, 30, 40]]) { + const result = mod.ownedUint8Array(input); + expect(result).toBeInstanceOf(Uint8Array); + expect(Array.from(result)).toEqual(input); } }); }); diff --git a/examples/js_dsl/mod.zig b/examples/js_dsl/mod.zig index 0d13d9c..8675323 100644 --- a/examples/js_dsl/mod.zig +++ b/examples/js_dsl/mod.zig @@ -12,6 +12,7 @@ const Object = js.Object; const Function = js.Function; const Value = js.Value; const Uint8Array = js.Uint8Array; +const OwnedUint8Array = js.OwnedUint8Array; const Float64Array = js.Float64Array; const Promise = js.Promise; @@ -233,6 +234,20 @@ pub fn externalUint8Array(arr: Array) !Uint8Array { return Uint8Array.fromExternal(tmp); } +/// Transfer an allocator-owned native allocation to a JavaScript Uint8Array. +pub fn ownedUint8Array(arr: Array) !OwnedUint8Array { + const len = try arr.length(); + const alloc = js.allocator(); + const data = try alloc.alloc(u8, len); + errdefer alloc.free(data); + + var i: u32 = 0; + while (i < len) : (i += 1) { + data[i] = @intCast((try arr.getNumber(i)).assertI32()); + } + return OwnedUint8Array.fromOwnedSlice(alloc, data); +} + // ============================================================================ // Section 7: Promises // ============================================================================ diff --git a/src/js.zig b/src/js.zig index 34af21a..3bbaef2 100644 --- a/src/js.zig +++ b/src/js.zig @@ -25,6 +25,7 @@ pub const Function = @import("js/function.zig").Function; pub const Value = @import("js/value.zig").Value; pub const TypedArray = typed_arrays.TypedArray; +pub const OwnedTypedArray = typed_arrays.OwnedTypedArray; pub const Int8Array = typed_arrays.Int8Array; pub const Uint8Array = typed_arrays.Uint8Array; pub const Uint8ClampedArray = typed_arrays.Uint8ClampedArray; @@ -36,6 +37,17 @@ pub const Float32Array = typed_arrays.Float32Array; pub const Float64Array = typed_arrays.Float64Array; pub const BigInt64Array = typed_arrays.BigInt64Array; pub const BigUint64Array = typed_arrays.BigUint64Array; +pub const OwnedInt8Array = typed_arrays.OwnedInt8Array; +pub const OwnedUint8Array = typed_arrays.OwnedUint8Array; +pub const OwnedUint8ClampedArray = typed_arrays.OwnedUint8ClampedArray; +pub const OwnedInt16Array = typed_arrays.OwnedInt16Array; +pub const OwnedUint16Array = typed_arrays.OwnedUint16Array; +pub const OwnedInt32Array = typed_arrays.OwnedInt32Array; +pub const OwnedUint32Array = typed_arrays.OwnedUint32Array; +pub const OwnedFloat32Array = typed_arrays.OwnedFloat32Array; +pub const OwnedFloat64Array = typed_arrays.OwnedFloat64Array; +pub const OwnedBigInt64Array = typed_arrays.OwnedBigInt64Array; +pub const OwnedBigUint64Array = typed_arrays.OwnedBigUint64Array; pub const Promise = @import("js/promise.zig").Promise; pub const createPromise = @import("js/promise.zig").createPromise; diff --git a/src/js/typed_arrays.zig b/src/js/typed_arrays.zig index 94fc5fc..672182f 100644 --- a/src/js/typed_arrays.zig +++ b/src/js/typed_arrays.zig @@ -60,72 +60,30 @@ pub fn TypedArray(comptime Element: type, comptime array_type: TypedarrayType) t /// native buffer is freed by a finalizer when V8 collects the ArrayBuffer. pub fn fromExternal(slice: []const Element) !Self { const e = context.env(); - const byte_len = slice.len * @sizeOf(Element); + const buf = try context.allocator().dupe(Element, slice); - const finalizer_context = try createFinalizerContext(context.allocator(), slice); - const arraybuffer = e.createExternalArrayBuffer( - std.mem.sliceAsBytes(finalizer_context.data), - externalFinalizer, - finalizer_context, - ) catch |err| { - // These statuses are returned before N-API installs the finalizer. - switch (err) { - error.NoExternalBuffersAllowed, - error.PendingException, - error.CannotRunJS, - => release(finalizer_context), - // Other failures may occur after the finalizer has taken ownership. - else => {}, - } + const byte_len = slice.len * @sizeOf(Element); + const len_hint: ?*anyopaque = @ptrFromInt(slice.len); + const finalize_cb = comptime napi.wrapSliceFinalizeCallback(Element, externalFinalizer); + const arraybuffer = e.createExternalArrayBuffer(std.mem.sliceAsBytes(buf), finalize_cb, len_hint) catch |err| { + context.allocator().free(buf); return err; }; _ = try e.adjustExternalMemory(@intCast(byte_len)); - finalizer_context.accounted = true; const val = try e.createTypedarray(array_type, slice.len, arraybuffer, 0); return .{ .val = val }; } - const FinalizerContext = struct { - allocator: std.mem.Allocator, - data: []Element, - accounted: bool = false, - }; - - fn createFinalizerContext( - allocator: std.mem.Allocator, - slice: []const Element, - ) !*FinalizerContext { - const data = try allocator.dupe(Element, slice); - errdefer allocator.free(data); - - const finalizer_context = try allocator.create(FinalizerContext); - finalizer_context.* = .{ - .allocator = allocator, - .data = data, - }; - return finalizer_context; - } - - fn externalFinalizer( - env: napi.c.napi_env, - _: ?*anyopaque, - finalize_hint: ?*anyopaque, - ) callconv(.c) void { - const finalizer_context: *FinalizerContext = - @ptrCast(@alignCast(finalize_hint orelse unreachable)); - if (finalizer_context.accounted) { - const byte_len = finalizer_context.data.len * @sizeOf(Element); - const e = napi.Env{ .env = env }; - _ = e.adjustExternalMemory(-@as(i64, @intCast(byte_len))) catch {}; - } - release(finalizer_context); - } - - fn release(finalizer_context: *FinalizerContext) void { - const allocator = finalizer_context.allocator; - allocator.free(finalizer_context.data); - allocator.destroy(finalizer_context); + /// Finalizer for buffers allocated by `fromExternal`. Frees the native + /// allocation and reverses the matching `adjustExternalMemory` accounting. + /// + /// Caller is responsible for calling a matching `adjustExternalMemory` at + /// the appropriate callsite to let V8 know about native heap memory usage. + fn externalFinalizer(env: napi.Env, data: []Element) void { + const byte_len = data.len * @sizeOf(Element); + context.allocator().free(data); + _ = env.adjustExternalMemory(-@as(i64, @intCast(byte_len))) catch {}; } /// Creates a new JavaScript TypedArray from a Zig slice by copying the data. @@ -176,6 +134,107 @@ pub fn TypedArray(comptime Element: type, comptime array_type: TypedarrayType) t }; } +/// Allocator-backed elements whose ownership can be transferred to a JavaScript +/// TypedArray without copying the element data. +/// +/// Like other Zig owning values, an OwnedTypedArray must not be copied or +/// deinitialized after transfer. +pub fn OwnedTypedArray(comptime Element: type, comptime array_type: TypedarrayType) type { + return struct { + allocator: std.mem.Allocator, + data: []Element, + + const Self = @This(); + pub const owned_typed_array = {}; + pub const expected_array_type = array_type; + + /// Takes ownership of `data`, which must have been allocated by `allocator`. + /// The allocator must remain valid until the value is deinitialized or + /// finalized by JavaScript. + pub fn fromOwnedSlice(allocator: std.mem.Allocator, data: []Element) Self { + return .{ + .allocator = allocator, + .data = data, + }; + } + + /// Copies `data` into a new owned allocation. + pub fn fromSlice(allocator: std.mem.Allocator, data: []const Element) !Self { + return .fromOwnedSlice(allocator, try allocator.dupe(Element, data)); + } + + /// Releases data that has not been transferred to JavaScript. + pub fn deinit(self: *Self) void { + self.allocator.free(self.data); + self.* = undefined; + } + + /// Transfers ownership to a JavaScript TypedArray. + /// + /// This consumes the value even on failure; the caller must not + /// deinitialize it afterwards. Unsupported external ArrayBuffers return + /// `error.NoExternalBuffersAllowed` without a copy fallback. + pub fn intoValue(self: Self, env: napi.Env) !napi.Value { + const allocator = self.allocator; + const data = self.data; + + if (data.len == 0) { + defer allocator.free(data); + const arraybuffer = try env.createArrayBuffer(0, null); + return env.createTypedarray(array_type, 0, arraybuffer, 0); + } + + const owner = try moveToHeap(self); + const arraybuffer = env.createExternalArrayBuffer( + std.mem.sliceAsBytes(data), + finalize, + owner, + ) catch |err| { + switch (err) { + error.NoExternalBuffersAllowed, + error.PendingException, + error.CannotRunJS, + => release(owner), + else => {}, + } + return err; + }; + + return env.createTypedarray(array_type, data.len, arraybuffer, 0); + } + + fn moveToHeap(self: Self) !*Self { + const owner = self.allocator.create(Self) catch |err| { + self.allocator.free(self.data); + return err; + }; + owner.* = self; + return owner; + } + + fn finalize( + _: napi.c.napi_env, + _: ?*anyopaque, + finalize_hint: ?*anyopaque, + ) callconv(.c) void { + const owner: *Self = + @ptrCast(@alignCast(finalize_hint orelse unreachable)); + release(owner); + } + + fn release(owner: *Self) void { + const allocator = owner.allocator; + allocator.free(owner.data); + allocator.destroy(owner); + } + }; +} + +/// Returns whether `T` is an OwnedTypedArray specialization. +pub fn isOwnedTypedArray(comptime T: type) bool { + return @typeInfo(T) == .@"struct" and @hasDecl(T, "owned_typed_array"); +} + // Concrete typed array types /// Wrapper around JavaScript `Int8Array`. pub const Int8Array = TypedArray(i8, .int8); @@ -210,19 +269,64 @@ pub const BigInt64Array = TypedArray(i64, .bigint64); /// Wrapper around JavaScript `BigUint64Array`. pub const BigUint64Array = TypedArray(u64, .biguint64); +/// Owned native elements transferable to a JavaScript `Int8Array`. +pub const OwnedInt8Array = OwnedTypedArray(i8, .int8); + +/// Owned native elements transferable to a JavaScript `Uint8Array`. +pub const OwnedUint8Array = OwnedTypedArray(u8, .uint8); + +/// Owned native elements transferable to a JavaScript `Uint8ClampedArray`. +pub const OwnedUint8ClampedArray = OwnedTypedArray(u8, .uint8_clamped); + +/// Owned native elements transferable to a JavaScript `Int16Array`. +pub const OwnedInt16Array = OwnedTypedArray(i16, .int16); + +/// Owned native elements transferable to a JavaScript `Uint16Array`. +pub const OwnedUint16Array = OwnedTypedArray(u16, .uint16); + +/// Owned native elements transferable to a JavaScript `Int32Array`. +pub const OwnedInt32Array = OwnedTypedArray(i32, .int32); + +/// Owned native elements transferable to a JavaScript `Uint32Array`. +pub const OwnedUint32Array = OwnedTypedArray(u32, .uint32); + +/// Owned native elements transferable to a JavaScript `Float32Array`. +pub const OwnedFloat32Array = OwnedTypedArray(f32, .float32); + +/// Owned native elements transferable to a JavaScript `Float64Array`. +pub const OwnedFloat64Array = OwnedTypedArray(f64, .float64); + +/// Owned native elements transferable to a JavaScript `BigInt64Array`. +pub const OwnedBigInt64Array = OwnedTypedArray(i64, .bigint64); + +/// Owned native elements transferable to a JavaScript `BigUint64Array`. +pub const OwnedBigUint64Array = OwnedTypedArray(u64, .biguint64); + test "TypedArray exposes expected subtype metadata" { try @import("std").testing.expect(Uint8Array.expected_array_type == .uint8); try @import("std").testing.expect(Float64Array.expected_array_type == .float64); } -test "TypedArray releases data when finalizer context allocation fails" { +test "OwnedTypedArray fromSlice owns an independent copy" { + var source = [_]u8{ 1, 2, 3 }; + var array = try OwnedUint8Array.fromSlice(std.testing.allocator, &source); + defer array.deinit(); + + source[0] = 9; + try std.testing.expectEqualSlices(u8, &.{ 1, 2, 3 }, array.data); +} + +test "OwnedTypedArray releases data when moving the owner to the heap fails" { + const data = try std.testing.allocator.dupe(u8, &.{ 1, 2, 3 }); + var failing_allocator = std.testing.FailingAllocator.init(std.testing.allocator, .{ - .fail_index = 1, + .fail_index = 0, }); + const array = OwnedUint8Array.fromOwnedSlice(failing_allocator.allocator(), data); try std.testing.expectError( error.OutOfMemory, - Uint8Array.createFinalizerContext(failing_allocator.allocator(), &.{ 1, 2, 3 }), + OwnedUint8Array.moveToHeap(array), ); try std.testing.expectEqual(@as(usize, 1), failing_allocator.deallocations); } diff --git a/src/js/wrap_function.zig b/src/js/wrap_function.zig index fcaf689..bd951aa 100644 --- a/src/js/wrap_function.zig +++ b/src/js/wrap_function.zig @@ -3,6 +3,7 @@ const napi = @import("../napi.zig"); const context = @import("context.zig"); const class_meta = @import("class_meta.zig"); const class_runtime = @import("class_runtime.zig"); +const typed_arrays = @import("typed_arrays.zig"); /// Checks whether `T` is a ZAPI DSL wrapper type (a struct with a `val: napi.Value` field). /// @@ -222,6 +223,14 @@ pub fn convertReturnWithCtor(comptime T: type, value: T, env: napi.c.napi_env, p if (T == napi.Value) { return value.value; } + if (comptime typed_arrays.isOwnedTypedArray(T)) { + const e = napi.Env{ .env = env }; + const result = value.intoValue(e) catch |err| { + e.throwError(@errorName(err), @errorName(err)) catch {}; + return null; + }; + return result.value; + } if (comptime isDslType(T)) { return value.val.value; } @@ -385,8 +394,6 @@ test "isDslType rejects non-DSL types" { } test "argTypeDescription names typed arrays and unwraps optionals" { - const typed_arrays = @import("typed_arrays.zig"); - try std.testing.expectEqualStrings("a number", argTypeDescription(?@import("number.zig").Number)); try std.testing.expectEqualStrings("a Uint8Array", argTypeDescription(typed_arrays.Uint8Array)); } From cf8a1fd6eaab3ffa6f280a9feb8010c7a98f0df9 Mon Sep 17 00:00:00 2001 From: Chen Kai <281165273grape@gmail.com> Date: Thu, 30 Jul 2026 11:31:44 +0800 Subject: [PATCH 6/6] fix: make owned typed array transfer transactional --- README.md | 9 ++-- src/js/typed_arrays.zig | 96 ++++++++++++++++++++++++++++++---------- src/js/wrap_function.zig | 4 +- 3 files changed, 81 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index de5a52f..0393ec8 100644 --- a/README.md +++ b/README.md @@ -301,10 +301,11 @@ pub fn serialize() !js.OwnedUint8Array { } ``` -Returning the value consumes it, including on conversion failure. JavaScript -releases the allocation through the ArrayBuffer finalizer, so the allocator must -remain valid until that finalizer runs. If external ArrayBuffers are unsupported, -the original error is returned; no copy fallback is performed. +Returning the value transfers its allocation to JavaScript without copying and +leaves the Zig owner empty. Failures before N-API accepts the external memory +leave ownership in Zig so it can be released normally. The allocator must remain +valid until the ArrayBuffer finalizer runs. If external ArrayBuffers are +unsupported, the original error is returned; no copy fallback is performed. ### Promises diff --git a/src/js/typed_arrays.zig b/src/js/typed_arrays.zig index 672182f..2d1bf37 100644 --- a/src/js/typed_arrays.zig +++ b/src/js/typed_arrays.zig @@ -134,12 +134,34 @@ pub fn TypedArray(comptime Element: type, comptime array_type: TypedarrayType) t }; } +fn elementType(comptime array_type: TypedarrayType) type { + return switch (array_type) { + .int8 => i8, + .uint8, .uint8_clamped => u8, + .int16 => i16, + .uint16 => u16, + .int32 => i32, + .uint32 => u32, + .float32 => f32, + .float64 => f64, + .bigint64 => i64, + .biguint64 => u64, + }; +} + /// Allocator-backed elements whose ownership can be transferred to a JavaScript /// TypedArray without copying the element data. /// -/// Like other Zig owning values, an OwnedTypedArray must not be copied or -/// deinitialized after transfer. +/// Like other Zig owning values, an OwnedTypedArray must not be copied. After +/// ownership transfers, the source is empty and may be deinitialized normally. pub fn OwnedTypedArray(comptime Element: type, comptime array_type: TypedarrayType) type { + if (Element != elementType(array_type)) { + @compileError( + "OwnedTypedArray element type `" ++ @typeName(Element) ++ + "` does not match `" ++ @tagName(array_type) ++ "`", + ); + } + return struct { allocator: std.mem.Allocator, data: []Element, @@ -163,7 +185,8 @@ pub fn OwnedTypedArray(comptime Element: type, comptime array_type: TypedarrayTy return .fromOwnedSlice(allocator, try allocator.dupe(Element, data)); } - /// Releases data that has not been transferred to JavaScript. + /// Releases data that has not been transferred to JavaScript. This is + /// also safe after a successful transfer, when `data` is empty. pub fn deinit(self: *Self) void { self.allocator.free(self.data); self.* = undefined; @@ -171,17 +194,22 @@ pub fn OwnedTypedArray(comptime Element: type, comptime array_type: TypedarrayTy /// Transfers ownership to a JavaScript TypedArray. /// - /// This consumes the value even on failure; the caller must not - /// deinitialize it afterwards. Unsupported external ArrayBuffers return - /// `error.NoExternalBuffersAllowed` without a copy fallback. - pub fn intoValue(self: Self, env: napi.Env) !napi.Value { - const allocator = self.allocator; + /// On success, `self.data` is empty and JavaScript releases the original + /// allocation through the ArrayBuffer finalizer. Failures before N-API + /// accepts the external memory leave ownership in `self`; failures after + /// ownership may have transferred leave `self.data` empty. + /// + /// Unsupported external ArrayBuffers return + /// `error.NoExternalBuffersAllowed` without a copy fallback. The caller + /// may deinitialize `self` after this function returns. + pub fn intoValue(self: *Self, env: napi.Env) !napi.Value { const data = self.data; if (data.len == 0) { - defer allocator.free(data); const arraybuffer = try env.createArrayBuffer(0, null); - return env.createTypedarray(array_type, 0, arraybuffer, 0); + const value = try env.createTypedarray(array_type, 0, arraybuffer, 0); + self.data = &.{}; + return value; } const owner = try moveToHeap(self); @@ -194,7 +222,7 @@ pub fn OwnedTypedArray(comptime Element: type, comptime array_type: TypedarrayTy error.NoExternalBuffersAllowed, error.PendingException, error.CannotRunJS, - => release(owner), + => restoreFromHeap(self, owner), else => {}, } return err; @@ -203,15 +231,20 @@ pub fn OwnedTypedArray(comptime Element: type, comptime array_type: TypedarrayTy return env.createTypedarray(array_type, data.len, arraybuffer, 0); } - fn moveToHeap(self: Self) !*Self { - const owner = self.allocator.create(Self) catch |err| { - self.allocator.free(self.data); - return err; - }; - owner.* = self; + fn moveToHeap(self: *Self) !*Self { + const owner = try self.allocator.create(Self); + owner.* = self.*; + self.data = &.{}; return owner; } + fn restoreFromHeap(self: *Self, owner: *Self) void { + const allocator = owner.allocator; + std.debug.assert(self.data.len == 0); + self.* = owner.*; + allocator.destroy(owner); + } + fn finalize( _: napi.c.napi_env, _: ?*anyopaque, @@ -316,17 +349,34 @@ test "OwnedTypedArray fromSlice owns an independent copy" { try std.testing.expectEqualSlices(u8, &.{ 1, 2, 3 }, array.data); } -test "OwnedTypedArray releases data when moving the owner to the heap fails" { - const data = try std.testing.allocator.dupe(u8, &.{ 1, 2, 3 }); - +test "OwnedTypedArray retains data when moving the owner to the heap fails" { var failing_allocator = std.testing.FailingAllocator.init(std.testing.allocator, .{ - .fail_index = 0, + .fail_index = 1, }); - const array = OwnedUint8Array.fromOwnedSlice(failing_allocator.allocator(), data); + var array = try OwnedUint8Array.fromSlice( + failing_allocator.allocator(), + &.{ 1, 2, 3 }, + ); try std.testing.expectError( error.OutOfMemory, - OwnedUint8Array.moveToHeap(array), + OwnedUint8Array.moveToHeap(&array), ); + try std.testing.expectEqualSlices(u8, &.{ 1, 2, 3 }, array.data); + + array.deinit(); try std.testing.expectEqual(@as(usize, 1), failing_allocator.deallocations); } + +test "OwnedTypedArray empties the source when ownership moves to the heap" { + var array = try OwnedUint8Array.fromSlice( + std.testing.allocator, + &.{ 1, 2, 3 }, + ); + const owner = try OwnedUint8Array.moveToHeap(&array); + defer OwnedUint8Array.release(owner); + + try std.testing.expectEqual(@as(usize, 0), array.data.len); + array.deinit(); + try std.testing.expectEqualSlices(u8, &.{ 1, 2, 3 }, owner.data); +} diff --git a/src/js/wrap_function.zig b/src/js/wrap_function.zig index bd951aa..48937fa 100644 --- a/src/js/wrap_function.zig +++ b/src/js/wrap_function.zig @@ -225,7 +225,9 @@ pub fn convertReturnWithCtor(comptime T: type, value: T, env: napi.c.napi_env, p } if (comptime typed_arrays.isOwnedTypedArray(T)) { const e = napi.Env{ .env = env }; - const result = value.intoValue(e) catch |err| { + var owned = value; + defer owned.deinit(); + const result = owned.intoValue(e) catch |err| { e.throwError(@errorName(err), @errorName(err)) catch {}; return null; };