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>
This commit is contained in:
Jarred Sumner
2023-07-30 02:03:32 -07:00
committed by GitHub
parent 413fd28120
commit 1db119ec11
10 changed files with 191 additions and 87 deletions

114
src/bun.js/Strong.zig Normal file
View File

@@ -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();
}
};

View File

@@ -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,

View File

@@ -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();

View File

@@ -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)
{

View File

@@ -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)

View File

@@ -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();
}

View File

@@ -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;
}
};

View File

@@ -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,

View File

@@ -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();

View File

@@ -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 {