diff --git a/README.md b/README.md index a76ef6e..09cec47 100644 --- a/README.md +++ b/README.md @@ -421,18 +421,40 @@ Control how arguments are converted: ```zig napi.createCallback(2, myFunc, .{ .args = .{ .env, .auto, .value, .data, .string, .buffer }, - .returns = .value, // or .string, .buffer, .auto + .returns = .value, // or .string, .buffer, .external_buffer, .auto }); ``` | Hint | Description | |------|-------------| -| `.auto` | Automatic type conversion | +| `.auto` | Automatic type conversion; byte-slice returns are copied into a JS Buffer | | `.env` | Inject `napi.Env` | | `.value` | Pass raw `napi.Value` | | `.data` | User data pointer passed to createFunction | | `.string` | Convert to/from `[]const u8` | -| `.buffer` | Convert to/from byte slice | +| `.buffer` | Borrow Buffer arguments as byte slices; copy byte-slice returns | +| `.external_buffer` | Transfer an `OwnedBuffer` to JavaScript without copying | + +External buffers require explicit ownership transfer: + +```zig +const allocator = std.heap.page_allocator; + +fn makeExternalBuffer() !napi.OwnedBuffer { + const data = try allocator.dupe(u8, "external data"); + return napi.OwnedBuffer.fromOwnedSlice(allocator, data); +} + +const callback = napi.createCallback(0, makeExternalBuffer, .{ + .returns = .external_buffer, +}); +``` + +`OwnedBuffer.intoValue` consumes the buffer even if conversion fails, so the caller must not +deinitialize it afterwards. Once N-API accepts the external buffer, the allocator must remain valid +until its finalizer runs, including if N-API subsequently reports an error. If the environment +disallows external buffers, `intoValue` copies the bytes and releases the original allocation before +returning. ### Creating Classes diff --git a/examples/hello_world/mod.test.ts b/examples/hello_world/mod.test.ts index 491e99c..a5b2ac6 100644 --- a/examples/hello_world/mod.test.ts +++ b/examples/hello_world/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 example = require("../../zig-out/lib/example_hello_world.node"); +const addonPath = require.resolve("../../zig-out/lib/example_hello_world.node"); +const example = require(addonPath); describe("example mod", () => { it("addon parameters", () => { @@ -36,10 +38,48 @@ describe("example mod", () => { expect(arrayBuffer.byteLength).toEqual(0); }); - it("shares mutable storage with an external Buffer", () => { + it("copies mutable slices returned without an external hint", () => { + const buffer = example.copySlice(); + buffer[0] = 9; + expect(example.copySlice()[0]).toEqual(1); + }); + + it("returns an allocator-owned external Buffer", () => { const buffer = example.externalBuffer(); expect(Buffer.isBuffer(buffer)).toBe(true); + expect([...buffer]).toEqual([1, 2, 3]); buffer[0] = 9; - expect(example.externalBufferFirstByte()).toEqual(9); + expect([...buffer]).toEqual([9, 2, 3]); + }); + + it("releases an external Buffer through its GC finalizer", () => { + const script = ` + const addon = require(${JSON.stringify(addonPath)}); + const baseline = addon.externalBufferAllocatedBytes(); + let buffer = addon.externalBuffer(); + if (addon.externalBufferAllocatedBytes() <= baseline) { + throw new Error("external Buffer was released while still reachable"); + } + buffer = null; + + const deadline = Date.now() + 5000; + function collect() { + global.gc(); + const allocatedBytes = addon.externalBufferAllocatedBytes(); + if (allocatedBytes === baseline) return; + if (Date.now() >= deadline) { + throw new Error( + \`external Buffer finalizer did not release \${allocatedBytes - baseline} bytes\`, + ); + } + setImmediate(collect); + } + setImmediate(collect); + `; + + execFileSync(process.execPath, ["--expose-gc", "-e", script], { + stdio: "pipe", + timeout: 10_000, + }); }); }); diff --git a/examples/hello_world/mod.zig b/examples/hello_world/mod.zig index 6d3807c..819a410 100644 --- a/examples/hello_world/mod.zig +++ b/examples/hello_world/mod.zig @@ -76,6 +76,13 @@ fn exampleMod(env: zapi.Env, module: zapi.Value) anyerror!void { null, )); + try module.setNamedProperty("copySlice", try env.createFunction( + "copySlice", + 0, + zapi.createCallback(0, copy_slice, .{}), + null, + )); + try module.setNamedProperty("externalBuffer", try env.createFunction( "externalBuffer", 0, @@ -85,10 +92,10 @@ fn exampleMod(env: zapi.Env, module: zapi.Value) anyerror!void { null, )); - try module.setNamedProperty("externalBufferFirstByte", try env.createFunction( - "externalBufferFirstByte", + try module.setNamedProperty("externalBufferAllocatedBytes", try env.createFunction( + "externalBufferAllocatedBytes", 0, - zapi.createCallback(0, external_buffer_first_byte, .{}), + zapi.createCallback(0, external_buffer_allocated_bytes, .{}), null, )); @@ -178,14 +185,26 @@ fn copy_buffer(env: zapi.Env, _: zapi.CallbackInfo(0)) !zapi.Value { return try env.createBufferCopy("copy me", null); } -var external_buffer_data = [_]u8{ 1, 2, 3 }; +const external_buffer_source = [_]u8{ 1, 2, 3 }; + +var slice_data = [_]u8{ 1, 2, 3 }; +var external_buffer_allocator: std.heap.DebugAllocator(.{ + .enable_memory_limit = true, + .thread_safe = false, +}) = .init; + +fn copy_slice() []u8 { + return &slice_data; +} -fn external_buffer() []u8 { - return &external_buffer_data; +fn external_buffer() !zapi.OwnedBuffer { + const owned_allocator = external_buffer_allocator.allocator(); + const data = try owned_allocator.dupe(u8, &external_buffer_source); + return zapi.OwnedBuffer.fromOwnedSlice(owned_allocator, data); } -fn external_buffer_first_byte() u8 { - return external_buffer_data[0]; +fn external_buffer_allocated_bytes() usize { + return external_buffer_allocator.total_requested_bytes; } const S = struct { diff --git a/src/OwnedBuffer.zig b/src/OwnedBuffer.zig new file mode 100644 index 0000000..b3bf0ae --- /dev/null +++ b/src/OwnedBuffer.zig @@ -0,0 +1,110 @@ +//! Allocator-backed bytes whose ownership can be transferred to JavaScript. +//! Like other Zig owning values, an OwnedBuffer must not be copied or deinitialized after transfer. + +const std = @import("std"); +const c = @import("c.zig").c; +const Env = @import("Env.zig"); +const Value = @import("Value.zig"); + +allocator: std.mem.Allocator, +data: []u8, + +const OwnedBuffer = @This(); + +const FinalizerContext = struct { + allocator: std.mem.Allocator, + data: []u8, +}; + +/// Takes ownership of `data`, which must have been allocated by `allocator`. +/// The allocator must remain valid until the buffer is deinitialized or finalized by JavaScript. +pub fn fromOwnedSlice(allocator: std.mem.Allocator, data: []u8) OwnedBuffer { + return .{ + .allocator = allocator, + .data = data, + }; +} + +/// Copies `data` into a new owned allocation. +pub fn fromSlice(allocator: std.mem.Allocator, data: []const u8) !OwnedBuffer { + return .fromOwnedSlice(allocator, try allocator.dupe(u8, data)); +} + +/// Releases a buffer that has not been transferred to JavaScript. +pub fn deinit(self: *OwnedBuffer) void { + self.allocator.free(self.data); + self.* = undefined; +} + +/// Transfers ownership to a JavaScript Buffer. +/// This consumes the buffer even on failure; the caller must not deinitialize it afterwards. +/// On success, JavaScript releases the allocation through the N-API finalizer. +pub fn intoValue(self: OwnedBuffer, env: Env) !Value { + const allocator = self.allocator; + const data = self.data; + + if (data.len == 0) { + defer allocator.free(data); + return try env.createBuffer(0, null); + } + + const context = try createFinalizerContext(self); + + return env.createExternalBuffer(data, finalize, context) catch |err| { + if (err == error.NoExternalBuffersAllowed) { + defer release(context); + return try env.createBufferCopy(data, null); + } + // Other failures may occur after the finalizer has taken ownership. + return err; + }; +} + +fn createFinalizerContext(self: OwnedBuffer) !*FinalizerContext { + const context = self.allocator.create(FinalizerContext) catch |err| { + self.allocator.free(self.data); + return err; + }; + context.* = .{ + .allocator = self.allocator, + .data = self.data, + }; + return context; +} + +fn finalize( + _: c.napi_env, + finalize_data: ?*anyopaque, + finalize_hint: ?*anyopaque, +) callconv(.c) void { + const context: *FinalizerContext = @ptrCast(@alignCast(finalize_hint orelse unreachable)); + std.debug.assert(finalize_data == @as(?*anyopaque, @ptrCast(context.data.ptr))); + release(context); +} + +fn release(context: *FinalizerContext) void { + const allocator = context.allocator; + allocator.free(context.data); + allocator.destroy(context); +} + +test "OwnedBuffer fromSlice owns an independent copy" { + var source = [_]u8{ 1, 2, 3 }; + var buffer = try OwnedBuffer.fromSlice(std.testing.allocator, &source); + defer buffer.deinit(); + + source[0] = 9; + try std.testing.expectEqualSlices(u8, &.{ 1, 2, 3 }, buffer.data); +} + +test "OwnedBuffer releases data when finalizer context allocation fails" { + const data = try std.testing.allocator.dupe(u8, "external"); + + var failing_allocator = std.testing.FailingAllocator.init(std.testing.allocator, .{ + .fail_index = 0, + }); + const buffer = OwnedBuffer.fromOwnedSlice(failing_allocator.allocator(), data); + + try std.testing.expectError(error.OutOfMemory, createFinalizerContext(buffer)); + try std.testing.expectEqual(@as(usize, 1), failing_allocator.deallocations); +} diff --git a/src/napi.zig b/src/napi.zig index bbba986..805d193 100644 --- a/src/napi.zig +++ b/src/napi.zig @@ -4,6 +4,7 @@ pub const c = @import("c.zig").c; pub const AsyncContext = @import("AsyncContext.zig"); pub const Env = @import("Env.zig"); pub const Value = @import("Value.zig"); +pub const OwnedBuffer = @import("OwnedBuffer.zig"); pub const Deferred = @import("Deferred.zig"); pub const EscapableHandleScope = @import("EscapableHandleScope.zig"); pub const HandleScope = @import("HandleScope.zig"); diff --git a/src/root.zig b/src/root.zig index af5e1e2..18c2869 100644 --- a/src/root.zig +++ b/src/root.zig @@ -7,6 +7,7 @@ pub const c = napi.c; pub const AsyncContext = napi.AsyncContext; pub const Env = napi.Env; pub const Value = napi.Value; +pub const OwnedBuffer = napi.OwnedBuffer; pub const Deferred = napi.Deferred; pub const EscapableHandleScope = napi.EscapableHandleScope; pub const HandleScope = napi.HandleScope; diff --git a/src/to_from_value.zig b/src/to_from_value.zig index 0c0d107..45bd71e 100644 --- a/src/to_from_value.zig +++ b/src/to_from_value.zig @@ -1,5 +1,6 @@ const std = @import("std"); const Env = @import("Env.zig"); +const OwnedBuffer = @import("OwnedBuffer.zig"); const Value = @import("Value.zig"); pub fn fromValue( @@ -55,6 +56,21 @@ pub fn toValue( comptime hint: anytype, ) !Value { const type_info = @typeInfo(T); + comptime { + const return_type = switch (type_info) { + .error_union => |error_union| error_union.payload, + else => T, + }; + if (hint == .external_buffer and return_type != OwnedBuffer) { + @compileError("external_buffer returns require an OwnedBuffer"); + } + if (return_type == OwnedBuffer and hint != .external_buffer) { + @compileError("OwnedBuffer returns require the external_buffer hint"); + } + } + + if (T == OwnedBuffer) return try v.intoValue(env); + switch (type_info) { .bool => { return try env.getBoolean(v); @@ -82,14 +98,6 @@ pub fn toValue( .pointer => |p| { const h = hint; if (p.child == u8 and p.size == .slice) { - if (h == .external_buffer) { - if (p.is_const) { - @compileError("external buffers require a mutable byte slice"); - } - const bytes: []u8 = @ptrCast(v); - return try env.createExternalBuffer(bytes, null, null); - } - const bytes: []const u8 = @ptrCast(v); if (h == .string) { return try env.createStringUtf8(bytes);