[socket] fix double-free in `finalize()` (#1731)

- tidy up `.isEmptyOrUndefinedOrNull()` usage

3 files changed, 45 insertions, 53 deletions

diff --git a/src/bun.js/api/bun/socket.zig b/src/bun.js/api/bun/socket.zig
index bded03f7f..3f6ea883a 100644
--- a/src/bun.js/api/bun/socket.zig
+++ b/src/bun.js/api/bun/socket.zig
@@ -119,7 +119,7 @@ const Handlers = struct {
         }
 
         const result = onError.callWithThis(this.globalObject, thisValue, err);
-        if (!result.isEmptyOrUndefinedOrNull() and result.isAnyError(this.globalObject)) {
+        if (result.isAnyError(this.globalObject)) {
             this.vm.onUnhandledError(this.globalObject, result);
         }
 
@@ -547,9 +547,7 @@ pub const Listener = struct {
         var listener = this.listener orelse return JSValue.jsUndefined();
         this.listener = null;
         listener.close(this.ssl);
-        if (this.poll_ref.isActive()) {
-            this.poll_ref.unref(this.handlers.vm);
-        }
+        this.poll_ref.unref(this.handlers.vm);
         if (this.handlers.active_connections == 0) {
             this.handlers.unprotect();
             this.socket_context.?.deinit(this.ssl);
@@ -619,8 +617,6 @@ pub const Listener = struct {
     }
 
     pub fn unref(this: *Listener, globalObject: *JSC.JSGlobalObject, _: *JSC.CallFrame) callconv(.C) JSValue {
-        if (!this.poll_ref.isActive()) return JSValue.jsUndefined();
-
         this.poll_ref.unref(globalObject.bunVM());
         if (this.handlers.active_connections == 0) {
             this.strong_self.clear();
@@ -857,7 +853,7 @@ fn NewSocket(comptime ssl: bool) type {
                 this_value,
             });
 
-            if (!result.isEmptyOrUndefinedOrNull() and result.isAnyError(globalObject)) {
+            if (result.isAnyError(globalObject)) {
                 _ = handlers.callErrorHandler(this_value, &[_]JSC.JSValue{ this_value, result });
             }
         }
@@ -883,7 +879,7 @@ fn NewSocket(comptime ssl: bool) type {
                 this_value,
             });
 
-            if (!result.isEmptyOrUndefinedOrNull() and result.isAnyError(globalObject)) {
+            if (result.isAnyError(globalObject)) {
                 _ = handlers.callErrorHandler(this_value, &[_]JSC.JSValue{ this_value, result });
             }
         }
@@ -967,7 +963,7 @@ fn NewSocket(comptime ssl: bool) type {
                 this_value,
             });
 
-            if (!result.isEmptyOrUndefinedOrNull() and result.isAnyError(globalObject)) {
+            if (result.isAnyError(globalObject)) {
                 this.detached = true;
                 defer this.markInactive();
                 if (!this.socket.isClosed()) {
@@ -1010,7 +1006,7 @@ fn NewSocket(comptime ssl: bool) type {
                 this_value,
             });
 
-            if (!result.isEmptyOrUndefinedOrNull() and result.isAnyError(globalObject)) {
+            if (result.isAnyError(globalObject)) {
                 _ = handlers.callErrorHandler(this_value, &[_]JSC.JSValue{ this_value, result });
             }
         }
@@ -1034,7 +1030,7 @@ fn NewSocket(comptime ssl: bool) type {
                 JSValue.jsNumber(@as(i32, err)),
             });
 
-            if (!result.isEmptyOrUndefinedOrNull() and result.isAnyError(globalObject)) {
+            if (result.isAnyError(globalObject)) {
                 _ = handlers.callErrorHandler(this_value, &[_]JSC.JSValue{ this_value, result });
             }
         }
@@ -1057,7 +1053,7 @@ fn NewSocket(comptime ssl: bool) type {
                 output_value,
             });
 
-            if (!result.isEmptyOrUndefinedOrNull() and result.isAnyError(globalObject)) {
+            if (result.isAnyError(globalObject)) {
                 _ = handlers.callErrorHandler(this_value, &[_]JSC.JSValue{ this_value, result });
             }
         }
@@ -1408,12 +1404,13 @@ fn NewSocket(comptime ssl: bool) type {
 
         pub fn finalize(this: *This) callconv(.C) void {
             log("finalize()", .{});
+            if (this.detached) return;
             this.detached = true;
             if (!this.socket.isClosed()) {
                 this.socket.close(0, null);
             }
             this.markInactive();
-            if (this.poll_ref.isActive()) this.poll_ref.unref(JSC.VirtualMachine.get());
+            this.poll_ref.unref(JSC.VirtualMachine.get());
         }
 
         pub fn reload(this: *This, globalObject: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) callconv(.C) JSValue {
diff --git a/src/bun.js/bindings/bindings.zig b/src/bun.js/bindings/bindings.zig
index 942d4b4f1..4da47ec70 100644
--- a/src/bun.js/bindings/bindings.zig
+++ b/src/bun.js/bindings/bindings.zig
@@ -2593,9 +2593,7 @@ pub const JSValue = enum(JSValueReprInt) {
             u16 => toU16(this),
             c_uint => @intCast(c_uint, toU32(this)),
             c_int => @intCast(c_int, toInt32(this)),
-            ?*JSInternalPromise => asInternalPromise(this),
-            ?*JSPromise => asPromise(this),
-
+            ?AnyPromise => asAnyPromise(this),
             u52 => @truncate(u52, @intCast(u64, @max(this.toInt64(), 0))),
             u64 => toUInt64NoTruncate(this),
             u8 => @truncate(u8, toU32(this)),
@@ -2804,7 +2802,6 @@ pub const JSValue = enum(JSValueReprInt) {
     pub fn asPromise(
         value: JSValue,
     ) ?*JSPromise {
-        if (value.isEmptyOrUndefinedOrNull()) return null;
         return cppFn("asPromise", .{
             value,
         });
@@ -2813,6 +2810,7 @@ pub const JSValue = enum(JSValueReprInt) {
     pub fn asAnyPromise(
         value: JSValue,
     ) ?AnyPromise {
+        if (value.isEmptyOrUndefinedOrNull()) return null;
         if (value.asInternalPromise()) |promise| {
             return AnyPromise{
                 .Internal = promise,
diff --git a/src/bun.js/test/jest.zig b/src/bun.js/test/jest.zig
index 07f802c13..590d5ec2b 100644
--- a/src/bun.js/test/jest.zig
+++ b/src/bun.js/test/jest.zig
@@ -1401,45 +1401,42 @@ pub const TestScope = struct {
             initial_value = js.JSObjectCallAsFunctionReturnValue(vm.global, callback, null, 0, null);
         }
 
-        if (!initial_value.isEmptyOrUndefinedOrNull()) {
-            if (initial_value.isAnyError(vm.global)) {
-                vm.runErrorHandler(initial_value, null);
-                return .{ .fail = active_test_expectation_counter.actual };
-            }
-
-            if (initial_value.asAnyPromise()) |promise| {
-                if (this.promise != null) {
-                    return .{ .pending = {} };
-                }
-                this.task = task;
+        if (initial_value.isAnyError(vm.global)) {
+            vm.runErrorHandler(initial_value, null);
+            return .{ .fail = active_test_expectation_counter.actual };
+        }
 
-                // TODO: not easy to coerce JSInternalPromise as JSValue,
-                // so simply wait for completion for now.
-                switch (promise) {
-                    .Internal => vm.waitForPromise(promise),
-                    else => {},
-                }
+        if (initial_value.asAnyPromise()) |promise| {
+            if (this.promise != null) {
+                return .{ .pending = {} };
+            }
+            this.task = task;
 
-                switch (promise.status(vm.global.vm())) {
-                    .Rejected => {
-                        vm.runErrorHandler(promise.result(vm.global.vm()), null);
-                        return .{ .fail = active_test_expectation_counter.actual };
-                    },
-                    .Pending => {
-                        task.promise_state = .pending;
-                        switch (promise) {
-                            .Normal => |p| {
-                                _ = p.asValue(vm.global).then(vm.global, task, onResolve, onReject);
-                                return .{ .pending = {} };
-                            },
-                            else => unreachable,
-                        }
-                    },
+            // TODO: not easy to coerce JSInternalPromise as JSValue,
+            // so simply wait for completion for now.
+            switch (promise) {
+                .Internal => vm.waitForPromise(promise),
+                else => {},
+            }
 
-                    else => {
-                        _ = promise.result(vm.global.vm());
-                    },
-                }
+            switch (promise.status(vm.global.vm())) {
+                .Rejected => {
+                    vm.runErrorHandler(promise.result(vm.global.vm()), null);
+                    return .{ .fail = active_test_expectation_counter.actual };
+                },
+                .Pending => {
+                    task.promise_state = .pending;
+                    switch (promise) {
+                        .Normal => |p| {
+                            _ = p.asValue(vm.global).then(vm.global, task, onResolve, onReject);
+                            return .{ .pending = {} };
+                        },
+                        else => unreachable,
+                    }
+                },
+                else => {
+                    _ = promise.result(vm.global.vm());
+                },
             }
         }