Fix that double free in Async

examples/memory/src/async.zig

@@ -1,15 +1,57 @@
 const std = @import("std");
 const napi = @import("napi");
 
+var custom_async_input_deinits = std.atomic.Value(usize).init(0);
+var custom_async_result_deinits = std.atomic.Value(usize).init(0);
+
 const AsyncInput = struct {
     label: []u8,
     values: []f32,
+
+    const Self = @This();
+
+    pub fn deinit(self: *Self, allocator: std.mem.Allocator) void {
+        allocator.free(self.label);
+        allocator.free(self.values);
+    }
 };
 
 const AsyncSummary = struct {
     label: []u8,
     count: usize,
     total: f64,
+
+    const Self = @This();
+
+    pub fn deinit(self: *Self, allocator: std.mem.Allocator) void {
+        allocator.free(self.label);
+    }
+};
+
+const CustomDeinitInput = struct {
+    owned_label: []u8,
+    borrowed_marker: []const u8,
+
+    const Self = @This();
+
+    pub fn deinit(self: *Self, allocator: std.mem.Allocator) void {
+        _ = custom_async_input_deinits.fetchAdd(1, .monotonic);
+        allocator.free(self.owned_label);
+    }
+};
+
+const CustomDeinitSummary = struct {
+    borrowed_input_marker: []const u8,
+    borrowed_result_marker: []const u8,
+    owned_label: []u8,
+    label_len: usize,
+
+    const Self = @This();
+
+    pub fn deinit(self: *Self, allocator: std.mem.Allocator) void {
+        _ = custom_async_result_deinits.fetchAdd(1, .monotonic);
+        allocator.free(self.owned_label);
+    }
 };
 
 const CountProgress = struct {
@@ -29,6 +71,16 @@ fn async_summary_execute(ctx: napi.AsyncContext(void), input: AsyncInput) !Async
     return .{ .label = label, .count = input.values.len, .total = total };
 }
 
+fn custom_deinit_execute(ctx: napi.AsyncContext(void), input: CustomDeinitInput) !CustomDeinitSummary {
+    const owned_label = try std.fmt.allocPrint(ctx.allocator, "{s}:owned", .{input.owned_label});
+    return .{
+        .borrowed_input_marker = input.borrowed_marker,
+        .borrowed_result_marker = "result-borrowed-marker",
+        .owned_label = owned_label,
+        .label_len = input.owned_label.len,
+    };
+}
+
 fn async_void_execute(_: []u8) void {}
 
 fn async_fail_execute(message: []u8) !void {
@@ -76,6 +128,33 @@ pub fn memory_async_summary_single(input: AsyncInput) napi.Async(AsyncSummary, .
     return napi.Async(AsyncSummary, .single).from(input, async_summary_execute);
 }
 
+pub fn memory_async_custom_deinit(label: []u8) napi.Async(CustomDeinitSummary, .thread) {
+    return napi.Async(CustomDeinitSummary, .thread).from(CustomDeinitInput{
+        .owned_label = label,
+        .borrowed_marker = "input-borrowed-marker",
+    }, custom_deinit_execute);
+}
+
+pub fn memory_async_custom_deinit_single(label: []u8) napi.Async(CustomDeinitSummary, .single) {
+    return napi.Async(CustomDeinitSummary, .single).from(CustomDeinitInput{
+        .owned_label = label,
+        .borrowed_marker = "input-borrowed-marker",
+    }, custom_deinit_execute);
+}
+
+pub fn memory_async_custom_deinit_reset() void {
+    custom_async_input_deinits.store(0, .monotonic);
+    custom_async_result_deinits.store(0, .monotonic);
+}
+
+pub fn memory_async_custom_input_deinit_count() usize {
+    return custom_async_input_deinits.load(.monotonic);
+}
+
+pub fn memory_async_custom_result_deinit_count() usize {
+    return custom_async_result_deinits.load(.monotonic);
+}
+
 pub fn memory_async_void(label: []u8) napi.Async(void, .thread) {
     return napi.Async(void, .thread).from(label, async_void_execute);
 }

examples/memory/src/hello.zig

@@ -74,6 +74,11 @@ pub const class_finalizer_count = classes.class_finalizer_count;
 
 pub const memory_async_summary = async_tests.memory_async_summary;
 pub const memory_async_summary_single = async_tests.memory_async_summary_single;
+pub const memory_async_custom_deinit = async_tests.memory_async_custom_deinit;
+pub const memory_async_custom_deinit_single = async_tests.memory_async_custom_deinit_single;
+pub const memory_async_custom_deinit_reset = async_tests.memory_async_custom_deinit_reset;
+pub const memory_async_custom_input_deinit_count = async_tests.memory_async_custom_input_deinit_count;
+pub const memory_async_custom_result_deinit_count = async_tests.memory_async_custom_result_deinit_count;
 pub const memory_async_void = async_tests.memory_async_void;
 pub const memory_async_fail = async_tests.memory_async_fail;
 pub const memory_async_progress = async_tests.memory_async_progress;

memory-testing/async.ts

@@ -83,6 +83,49 @@ export async function exerciseAsyncWrappers(native: NativeAddon) {
   assertEqual(singleSummary.count, 3, "async single count");
   assertEqual(singleSummary.total, 10, "async single total");
 
+  native.memory_async_custom_deinit_reset();
+  const customSummary = await native.memory_async_custom_deinit("custom-thread");
+  assertEqual(
+    customSummary.borrowed_input_marker,
+    "input-borrowed-marker",
+    "custom deinit borrowed input marker",
+  );
+  assertEqual(
+    customSummary.borrowed_result_marker,
+    "result-borrowed-marker",
+    "custom deinit borrowed result marker",
+  );
+  assertEqual(customSummary.owned_label, "custom-thread:owned", "custom deinit owned label");
+  assertEqual(customSummary.label_len, 13, "custom deinit label length");
+
+  const customSingleSummary = await native.memory_async_custom_deinit_single("custom-single");
+  assertEqual(
+    customSingleSummary.borrowed_input_marker,
+    "input-borrowed-marker",
+    "custom single deinit borrowed input marker",
+  );
+  assertEqual(
+    customSingleSummary.borrowed_result_marker,
+    "result-borrowed-marker",
+    "custom single deinit borrowed result marker",
+  );
+  assertEqual(
+    customSingleSummary.owned_label,
+    "custom-single:owned",
+    "custom single deinit owned label",
+  );
+  assertEqual(customSingleSummary.label_len, 13, "custom single deinit label length");
+  assertEqual(
+    native.memory_async_custom_input_deinit_count(),
+    2,
+    "custom async input deinit count",
+  );
+  assertEqual(
+    native.memory_async_custom_result_deinit_count(),
+    2,
+    "custom async result deinit count",
+  );
+
   await native.memory_async_void("async-void");
   await assertRejects(
     native.memory_async_fail("async memory failure"),

memory-testing/finalizers.ts

@@ -3,7 +3,7 @@ import { assertEqual } from "./assert";
 type NativeAddon = ESObject;
 
 export function exerciseFinalizerWrappers(native: NativeAddon) {
-  native.begin_finalizer_state_check(128, 96);
+  native.begin_finalizer_state_check(128, 128);
 
   for (let i = 0; i < 32; i++) {
     let bufferValue: ESObject | null = native.create_external_buffer(32);

scripts/arkvm/run_arkvm_memory_tests.sh

@@ -10,7 +10,7 @@ export ARKVM_RESULT_PREFIX="${ARKVM_RESULT_PREFIX:-__ZIG_NAPI_MEMORY_RESULT__}"
 export ARKVM_EXAMPLE_DIR="${ARKVM_EXAMPLE_DIR:-examples/memory}"
 export ARKVM_WORK_ROOT="${ARKVM_WORK_ROOT:-${REPO_ROOT}/.tmp_arkvm_memory_runner}"
 export ARKVM_WAIT_FOR_EXIT="${ARKVM_WAIT_FOR_EXIT:-1}"
-export ARKVM_EXPECT_LOG="${ARKVM_EXPECT_LOG:-^__ZIG_NAPI_FINALIZER_RESULT__ status=ok external=128 class=96$}"
+export ARKVM_EXPECT_LOG="${ARKVM_EXPECT_LOG:-^__ZIG_NAPI_FINALIZER_RESULT__ status=ok external=128 class=128$}"
 export TEST_TIMEOUT_SEC="${TEST_TIMEOUT_SEC:-180}"
 
 exec "${SCRIPT_DIR}/run_arkvm_tests.sh"

src/napi/async.zig

@@ -7,6 +7,7 @@ const Undefined = @import("./value/undefined.zig").Undefined;
 const Napi = @import("./util/napi.zig").Napi;
 const NapiError = @import("./wrapper/error.zig");
 const GlobalAllocator = @import("./util/allocator.zig");
+const AbortSignalModule = @import("./abort_signal.zig");
 const AbortSignal = @import("./abort_signal.zig").AbortSignal;
 const AbortRegistration = @import("./abort_signal.zig").AbortRegistration;
 
@@ -624,17 +625,31 @@ fn AsyncTaskOperation(
                 self.controller_thread = null;
             }
 
+            const promise = self.promise;
+            var settled_value: ?napi.napi_value = null;
+            var should_reject = false;
+
             if (self.cancel_dispatched or self.cancel_requested) {
-                self.promise.RejectAbortError() catch {};
+                settled_value = AbortSignalModule.abortErrorValue(Env.from_raw(env_raw));
+                should_reject = true;
             } else if (self.err) |err| {
-                self.promise.Reject(err) catch {};
+                settled_value = err.to_napi_error(Env.from_raw(env_raw));
+                should_reject = true;
             } else if (Result == void) {
-                self.promise.Resolve({}) catch {};
+                settled_value = Undefined.New(Env.from_raw(env_raw)).raw;
             } else {
-                self.promise.Resolve(self.result) catch {};
+                settled_value = Napi.to_napi_value(env_raw, self.result, null) catch null;
             }
 
             self.destroy(env_raw);
+
+            if (settled_value) |value| {
+                const status = if (should_reject)
+                    napi.napi_reject_deferred(env_raw, promise.deferred, value)
+                else
+                    napi.napi_resolve_deferred(env_raw, promise.deferred, value);
+                _ = status;
+            }
         }
 
         fn initThreadDispatcher(self: *Self) !void {
@@ -682,17 +697,21 @@ fn AsyncTaskOperation(
             const self: *Self = @ptrCast(@alignCast(context));
             const data: *DispatchData = @ptrCast(@alignCast(raw_data));
             const allocator = self.allocator;
-            defer allocator.destroy(data);
 
             switch (data.kind) {
                 .event => {
+                    defer allocator.destroy(data);
                     if (Event != void and data.payload != null) {
                         const payload = data.payload.?;
                         defer allocator.destroy(payload);
                         self.dispatchEvent(inner_env, payload.*);
                     }
                 },
-                .completion => self.dispatchCompletion(inner_env),
+                .completion => {
+                    allocator.destroy(data);
+                    self.dispatchCompletion(inner_env);
+                    return;
+                },
             }
         }
 

src/napi/util/napi.zig

@@ -207,6 +207,52 @@ pub const Napi = struct {
         Napi.deinit_napi_value_with_state(T, value, &state);
     }
 
+    fn deinitWithCustomStructDeinit(comptime T: type, value: T, allocator: std.mem.Allocator) bool {
+        if (!@hasDecl(T, "deinit")) return false;
+
+        const deinit_fn = @field(T, "deinit");
+        const deinit_info = @typeInfo(@TypeOf(deinit_fn));
+        if (deinit_info != .@"fn") {
+            @compileError("Struct " ++ @typeName(T) ++ ".deinit must be a function");
+        }
+
+        const params = deinit_info.@"fn".params;
+        if (params.len == 0 or params.len > 2) {
+            @compileError("Struct " ++ @typeName(T) ++ ".deinit must accept (self) or (self, allocator)");
+        }
+
+        const self_type = params[0].type orelse {
+            @compileError("Struct " ++ @typeName(T) ++ ".deinit self parameter must be typed");
+        };
+        const self_info = @typeInfo(self_type);
+        const valid_self_type = self_type == T or
+            (self_info == .pointer and self_info.pointer.size == .one and self_info.pointer.child == T);
+        if (!valid_self_type) {
+            @compileError("Struct " ++ @typeName(T) ++ ".deinit first parameter must be Self, *Self, or *const Self");
+        }
+
+        const return_type = deinit_info.@"fn".return_type orelse void;
+        if (return_type != void) {
+            @compileError("Struct " ++ @typeName(T) ++ ".deinit must return void");
+        }
+
+        var mutable = value;
+        if (params.len == 1) {
+            mutable.deinit();
+            return true;
+        }
+
+        const allocator_type = params[1].type orelse {
+            @compileError("Struct " ++ @typeName(T) ++ ".deinit allocator parameter must be typed");
+        };
+        if (allocator_type != std.mem.Allocator) {
+            @compileError("Struct " ++ @typeName(T) ++ ".deinit allocator parameter must be std.mem.Allocator");
+        }
+
+        mutable.deinit(allocator);
+        return true;
+    }
+
     pub fn deinit_napi_value_with_state(comptime T: type, value: T, state: *DeinitState) void {
         const allocator = GlobalAllocator.globalAllocator();
         const infos = @typeInfo(T);
@@ -276,6 +322,10 @@ pub const Napi = struct {
                     return;
                 }
 
+                if (Napi.deinitWithCustomStructDeinit(T, value, allocator)) {
+                    return;
+                }
+
                 inline for (infos.@"struct".fields) |field| {
                     Napi.deinit_napi_value_with_state(field.type, @field(value, field.name), state);
                 }

src/napi/wrapper/class.zig

@@ -53,11 +53,7 @@ pub fn ClassWrapper(comptime T: type, comptime HasInit: bool) type {
                 GlobalAllocator.global_manager.set(self.allocator);
                 defer GlobalAllocator.global_manager.set(previous_allocator);
 
-                if (@hasDecl(T, "deinit")) {
-                    self.value.deinit();
-                } else {
-                    Napi.deinit_napi_value(T, self.value);
-                }
+                Napi.deinit_napi_value(T, self.value);
                 self.allocator.destroy(self);
             }
         };