Fix memory leak (#3887)

* Fix memory leak

* Remove an extra copy

* Further fixes

---------

Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>

10 files changed, 191 insertions, 87 deletions

diff --git a/src/bun.js/Strong.zig b/src/bun.js/Strong.zig
new file mode 100644
index 000000000..d2ed3afbd
--- /dev/null
+++ b/src/bun.js/Strong.zig
@@ -0,0 +1,114 @@
+const bun = @import("root").bun;
+const JSC = bun.JSC;
+
+const StrongImpl = opaque {
+    pub fn init(globalThis: *JSC.JSGlobalObject, value: JSC.JSValue) *StrongImpl {
+        JSC.markBinding(@src());
+        return Bun__StrongRef__new(globalThis, value);
+    }
+
+    pub fn get(this: *StrongImpl) JSC.JSValue {
+        JSC.markBinding(@src());
+        return Bun__StrongRef__get(this);
+    }
+
+    pub fn set(this: *StrongImpl, globalThis: *JSC.JSGlobalObject, value: JSC.JSValue) void {
+        JSC.markBinding(@src());
+        Bun__StrongRef__set(this, globalThis, value);
+    }
+
+    pub fn clear(this: *StrongImpl) void {
+        JSC.markBinding(@src());
+        Bun__StrongRef__clear(this);
+    }
+
+    pub fn deinit(
+        this: *StrongImpl,
+    ) void {
+        JSC.markBinding(@src());
+        Bun__StrongRef__delete(this);
+    }
+
+    extern fn Bun__StrongRef__delete(this: *StrongImpl) void;
+    extern fn Bun__StrongRef__new(*JSC.JSGlobalObject, JSC.JSValue) *StrongImpl;
+    extern fn Bun__StrongRef__get(this: *StrongImpl) JSC.JSValue;
+    extern fn Bun__StrongRef__set(this: *StrongImpl, *JSC.JSGlobalObject, JSC.JSValue) void;
+    extern fn Bun__StrongRef__clear(this: *StrongImpl) void;
+};
+
+pub const Strong = struct {
+    ref: ?*StrongImpl = null,
+    globalThis: ?*JSC.JSGlobalObject = null,
+
+    pub fn init() Strong {
+        return .{};
+    }
+
+    pub fn create(
+        value: JSC.JSValue,
+        globalThis: *JSC.JSGlobalObject,
+    ) Strong {
+        if (value != .zero) {
+            return .{ .ref = StrongImpl.init(globalThis, value), .globalThis = globalThis };
+        }
+
+        return .{ .globalThis = globalThis };
+    }
+
+    pub fn get(this: *Strong) ?JSC.JSValue {
+        var ref = this.ref orelse return null;
+        const result = ref.get();
+        if (result == .zero) {
+            return null;
+        }
+
+        return result;
+    }
+
+    pub fn swap(this: *Strong) JSC.JSValue {
+        var ref = this.ref orelse return .zero;
+        const result = ref.get();
+        if (result == .zero) {
+            return .zero;
+        }
+
+        ref.clear();
+        return result;
+    }
+
+    pub fn has(this: *Strong) bool {
+        var ref = this.ref orelse return false;
+        return ref.get() != .zero;
+    }
+
+    pub fn trySwap(this: *Strong) ?JSC.JSValue {
+        const result = this.swap();
+        if (result == .zero) {
+            return null;
+        }
+
+        return result;
+    }
+
+    pub fn set(this: *Strong, globalThis: *JSC.JSGlobalObject, value: JSC.JSValue) void {
+        var ref: *StrongImpl = this.ref orelse {
+            if (value == .zero) return;
+            this.ref = StrongImpl.init(globalThis, value);
+            this.globalThis = globalThis;
+            return;
+        };
+        this.globalThis = globalThis;
+        ref.set(globalThis, value);
+    }
+
+    pub fn clear(this: *Strong) void {
+        var ref: *StrongImpl = this.ref orelse return;
+        ref.clear();
+    }
+
+    pub fn deinit(this: *Strong) void {
+        var ref: *StrongImpl = this.ref orelse return;
+        this.ref = null;
+        ref.deinit();
+    }
+};
diff --git a/src/bun.js/base.zig b/src/bun.js/base.zig
index 579a0975a..c964c1d95 100644
--- a/src/bun.js/base.zig
+++ b/src/bun.js/base.zig
@@ -3967,78 +3967,7 @@ pub const FilePoll = struct {
     }
 };
 
-pub const Strong = extern struct {
-    ref: ?*JSC.napi.Ref = null,
-
-    pub fn init() Strong {
-        return .{};
-    }
-
-    pub fn create(
-        value: JSC.JSValue,
-        globalThis: *JSC.JSGlobalObject,
-    ) Strong {
-        var str = Strong.init();
-        if (value != .zero)
-            str.set(globalThis, value);
-        return str;
-    }
-
-    pub fn get(this: *Strong) ?JSValue {
-        var ref = this.ref orelse return null;
-        const result = ref.get();
-        if (result == .zero) {
-            return null;
-        }
-
-        return result;
-    }
-
-    pub fn swap(this: *Strong) JSValue {
-        var ref = this.ref orelse return .zero;
-        const result = ref.get();
-        if (result == .zero) {
-            return .zero;
-        }
-
-        ref.set(.zero);
-        return result;
-    }
-
-    pub fn has(this: *Strong) bool {
-        var ref = this.ref orelse return false;
-        return ref.get() != .zero;
-    }
-
-    pub fn trySwap(this: *Strong) ?JSValue {
-        const result = this.swap();
-        if (result == .zero) {
-            return null;
-        }
-
-        return result;
-    }
-
-    pub fn set(this: *Strong, globalThis: *JSC.JSGlobalObject, value: JSValue) void {
-        var ref: *JSC.napi.Ref = this.ref orelse {
-            if (value == .zero) return;
-            this.ref = JSC.napi.Ref.create(globalThis, value);
-            return;
-        };
-        ref.set(value);
-    }
-
-    pub fn clear(this: *Strong) void {
-        var ref: *JSC.napi.Ref = this.ref orelse return;
-        ref.set(JSC.JSValue.zero);
-    }
-
-    pub fn deinit(this: *Strong) void {
-        var ref: *JSC.napi.Ref = this.ref orelse return;
-        this.ref = null;
-        ref.destroy();
-    }
-};
+pub const Strong = @import("./Strong.zig").Strong;
 
 pub const BinaryType = enum {
     Buffer,
diff --git a/src/bun.js/bindings/JSCTaskScheduler.cpp b/src/bun.js/bindings/JSCTaskScheduler.cpp
index 436be4c0a..b97b2e9d2 100644
--- a/src/bun.js/bindings/JSCTaskScheduler.cpp
+++ b/src/bun.js/bindings/JSCTaskScheduler.cpp
@@ -24,9 +24,11 @@ public:
     Ticket ticket;
     Task task;
 
-    WTF_MAKE_FAST_ALLOCATED;
+    WTF_MAKE_ISO_ALLOCATED(JSCDeferredWorkTask);
 };
 
+WTF_MAKE_ISO_ALLOCATED_IMPL(JSCDeferredWorkTask);
+
 static JSC::VM& getVM(Ticket ticket)
 {
     return ticket->scriptExecutionOwner.get()->vm();
diff --git a/src/bun.js/bindings/ScriptExecutionContext.cpp b/src/bun.js/bindings/ScriptExecutionContext.cpp
index d9adbeb98..eab41d584 100644
--- a/src/bun.js/bindings/ScriptExecutionContext.cpp
+++ b/src/bun.js/bindings/ScriptExecutionContext.cpp
@@ -14,6 +14,9 @@ namespace WebCore {
 
 static std::atomic<unsigned> lastUniqueIdentifier = 0;
 
+WTF_MAKE_ISO_ALLOCATED_IMPL(EventLoopTask);
+WTF_MAKE_ISO_ALLOCATED_IMPL(ScriptExecutionContext);
+
 static Lock allScriptExecutionContextsMapLock;
 static HashMap<ScriptExecutionContextIdentifier, ScriptExecutionContext*>& allScriptExecutionContextsMap() WTF_REQUIRES_LOCK(allScriptExecutionContextsMapLock)
 {
diff --git a/src/bun.js/bindings/ScriptExecutionContext.h b/src/bun.js/bindings/ScriptExecutionContext.h
index 23c44ab51..271c7e205 100644
--- a/src/bun.js/bindings/ScriptExecutionContext.h
+++ b/src/bun.js/bindings/ScriptExecutionContext.h
@@ -34,7 +34,7 @@ class MessagePort;
 class ScriptExecutionContext;
 
 class EventLoopTask {
-    WTF_MAKE_FAST_ALLOCATED;
+    WTF_MAKE_ISO_ALLOCATED(EventLoopTask);
 
 public:
     enum CleanupTaskTag { CleanupTask };
@@ -74,6 +74,7 @@ protected:
 using ScriptExecutionContextIdentifier = uint32_t;
 
 class ScriptExecutionContext : public CanMakeWeakPtr<ScriptExecutionContext> {
+    WTF_MAKE_ISO_ALLOCATED(ScriptExecutionContext);
 
 public:
     ScriptExecutionContext(JSC::VM* vm, JSC::JSGlobalObject* globalObject)
diff --git a/src/bun.js/bindings/Strong.cpp b/src/bun.js/bindings/Strong.cpp
new file mode 100644
index 000000000..8ec63e318
--- /dev/null
+++ b/src/bun.js/bindings/Strong.cpp
@@ -0,0 +1,53 @@
+#include "root.h"
+#include <JavaScriptCore/StrongInlines.h>
+#include "BunClientData.h"
+
+namespace Bun {
+
+// We tried to pool these
+// But it was very complicated
+class StrongRef {
+    WTF_MAKE_ISO_ALLOCATED(StrongRef);
+
+public:
+    StrongRef(JSC::VM& vm, JSC::JSValue value)
+        : m_cell(vm, value)
+    {
+    }
+
+    StrongRef()
+        : m_cell()
+    {
+    }
+
+    JSC::Strong<JSC::Unknown> m_cell;
+};
+
+WTF_MAKE_ISO_ALLOCATED_IMPL(StrongRef);
+
+}
+
+extern "C" void Bun__StrongRef__delete(Bun::StrongRef* strongRef)
+{
+    delete strongRef;
+}
+
+extern "C" Bun::StrongRef* Bun__StrongRef__new(JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue encodedValue)
+{
+    return new Bun::StrongRef(globalObject->vm(), JSC::JSValue::decode(encodedValue));
+}
+
+extern "C" JSC::EncodedJSValue Bun__StrongRef__get(Bun::StrongRef* strongRef)
+{
+    return JSC::JSValue::encode(strongRef->m_cell.get());
+}
+
+extern "C" void Bun__StrongRef__set(Bun::StrongRef* strongRef, JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue value)
+{
+    strongRef->m_cell.set(globalObject->vm(), JSC::JSValue::decode(value));
+}
+
+extern "C" void Bun__StrongRef__clear(Bun::StrongRef* strongRef)
+{
+    strongRef->m_cell.clear();
+}
diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp
index ffd6e990a..3776adb2b 100644
--- a/src/bun.js/bindings/ZigGlobalObject.cpp
+++ b/src/bun.js/bindings/ZigGlobalObject.cpp
@@ -227,11 +227,11 @@ public:
         if (encodedString.isNull())
             return String();
 
-        auto decodedData = base64Decode(encodedString, Base64DecodeMode::DefaultValidatePaddingAndIgnoreWhitespace);
+        auto decodedData = base64DecodeToString(encodedString, Base64DecodeMode::DefaultValidatePaddingAndIgnoreWhitespace);
         if (!decodedData)
             return Exception { InvalidCharacterError };
 
-        return String(decodedData->data(), decodedData->size());
+        return decodedData;
     }
 };
 
diff --git a/src/bun.js/bindings/napi.cpp b/src/bun.js/bindings/napi.cpp
index 6e47db8c4..c27bcf533 100644
--- a/src/bun.js/bindings/napi.cpp
+++ b/src/bun.js/bindings/napi.cpp
@@ -170,6 +170,8 @@ typedef struct StackAllocatedCallFrame {
 extern "C" Zig::GlobalObject*
 Bun__getDefaultGlobal();
 
+WTF_MAKE_ISO_ALLOCATED_IMPL(NapiRef);
+
 static uint32_t getPropertyAttributes(napi_property_attributes attributes)
 {
     uint32_t result = 0;
@@ -951,7 +953,7 @@ extern "C" napi_status napi_delete_reference(napi_env env, napi_ref ref)
 extern "C" void napi_delete_reference_internal(napi_ref ref)
 {
     NapiRef* napiRef = toJS(ref);
-    napiRef->~NapiRef();
+    delete napiRef;
 }
 
 extern "C" napi_status napi_is_detached_arraybuffer(napi_env env,
diff --git a/src/bun.js/bindings/napi.h b/src/bun.js/bindings/napi.h
index 874377cf0..550803963 100644
--- a/src/bun.js/bindings/napi.h
+++ b/src/bun.js/bindings/napi.h
@@ -62,7 +62,7 @@ public:
 };
 
 class NapiRef : public RefCounted<NapiRef>, public CanMakeWeakPtr<NapiRef> {
-    WTF_MAKE_FAST_ALLOCATED;
+    WTF_MAKE_ISO_ALLOCATED(NapiRef);
 
 public:
     void ref();
diff --git a/src/napi/napi.zig b/src/napi/napi.zig
index abe8316ad..9d361a14e 100644
--- a/src/napi/napi.zig
+++ b/src/napi/napi.zig
@@ -794,7 +794,7 @@ pub export fn napi_get_dataview_info(env: napi_env, dataview: napi_value, bytele
     var array_buffer = dataview.asArrayBuffer(env) orelse return .object_expected;
     bytelength.* = array_buffer.byte_len;
     data.* = array_buffer.ptr;
-    // TODO: will this work? will it fail due to being a DataView instead of a TypedArray?
+
     arraybuffer.* = JSValue.c(JSC.C.JSObjectGetTypedArrayBuffer(env.ref(), dataview.asObjectRef(), null));
     byte_offset.* = array_buffer.offset;
     return .ok;
@@ -806,24 +806,24 @@ pub export fn napi_get_version(_: napi_env, result: *u32) napi_status {
 }
 pub export fn napi_create_promise(env: napi_env, deferred: *napi_deferred, promise: *napi_value) napi_status {
     log("napi_create_promise", .{});
-    deferred.* = JSC.JSPromise.Strong.init(env).strong.ref.?;
-    promise.* = deferred.*.get();
+    var js_promise = JSC.JSPromise.create(env);
+    var promise_value = js_promise.asValue(env);
+    deferred.* = Ref.create(env, promise_value);
+    promise.* = promise_value;
     return .ok;
 }
 pub export fn napi_resolve_deferred(env: napi_env, deferred: napi_deferred, resolution: napi_value) napi_status {
     log("napi_resolve_deferred", .{});
-    var prom = JSC.JSPromise.Strong{
-        .strong = .{ .ref = deferred },
-    };
+    var prom = deferred.get().asPromise() orelse return .object_expected;
     prom.resolve(env, resolution);
+    deferred.destroy();
     return .ok;
 }
 pub export fn napi_reject_deferred(env: napi_env, deferred: napi_deferred, rejection: napi_value) napi_status {
     log("napi_reject_deferred", .{});
-    var prom = JSC.JSPromise.Strong{
-        .strong = .{ .ref = deferred },
-    };
+    var prom = deferred.get().asPromise() orelse return .object_expected;
     prom.reject(env, rejection);
+    deferred.destroy();
     return .ok;
 }
 pub export fn napi_is_promise(_: napi_env, value: napi_value, is_promise: *bool) napi_status {