diff --git a/src/bun.js/api/FFI.h b/src/bun.js/api/FFI.h index 6616e5de47..a6d6e88c86 100644 --- a/src/bun.js/api/FFI.h +++ b/src/bun.js/api/FFI.h @@ -60,8 +60,8 @@ typedef enum { napi_detachable_arraybuffer_expected, napi_would_deadlock // unused } napi_status; -void* NapiHandleScope__push(void* jsGlobalObject, bool detached); -void NapiHandleScope__pop(void* jsGlobalObject, void* handleScope); +void* NapiHandleScope__open(void* jsGlobalObject, bool detached); +void NapiHandleScope__close(void* jsGlobalObject, void* handleScope); #endif diff --git a/src/bun.js/api/ffi.zig b/src/bun.js/api/ffi.zig index 858595cac2..e0d3a2327d 100644 --- a/src/bun.js/api/ffi.zig +++ b/src/bun.js/api/ffi.zig @@ -1800,7 +1800,7 @@ pub const FFI = struct { if (this.needsHandleScope()) { try writer.writeAll( - \\ void* handleScope = NapiHandleScope__push(JS_GLOBAL_OBJECT, false); + \\ void* handleScope = NapiHandleScope__open(JS_GLOBAL_OBJECT, false); \\ ); } @@ -1913,7 +1913,7 @@ pub const FFI = struct { if (this.needsHandleScope()) { try writer.writeAll( - \\ NapiHandleScope__pop(JS_GLOBAL_OBJECT, handleScope); + \\ NapiHandleScope__close(JS_GLOBAL_OBJECT, handleScope); \\ ); } @@ -2487,8 +2487,8 @@ const CompilerRT = struct { pub fn inject(state: *TCC.TCCState) void { _ = TCC.tcc_add_symbol(state, "memset", &memset); _ = TCC.tcc_add_symbol(state, "memcpy", &memcpy); - _ = TCC.tcc_add_symbol(state, "NapiHandleScope__push", &bun.JSC.napi.NapiHandleScope.NapiHandleScope__push); - _ = TCC.tcc_add_symbol(state, "NapiHandleScope__pop", &bun.JSC.napi.NapiHandleScope.NapiHandleScope__pop); + _ = TCC.tcc_add_symbol(state, "NapiHandleScope__open", &bun.JSC.napi.NapiHandleScope.NapiHandleScope__open); + _ = TCC.tcc_add_symbol(state, "NapiHandleScope__close", &bun.JSC.napi.NapiHandleScope.NapiHandleScope__close); _ = TCC.tcc_add_symbol( state, diff --git a/src/bun.js/bindings/napi_handle_scope.cpp b/src/bun.js/bindings/napi_handle_scope.cpp index b23d8bdfde..ed3af6e618 100644 --- a/src/bun.js/bindings/napi_handle_scope.cpp +++ b/src/bun.js/bindings/napi_handle_scope.cpp @@ -78,7 +78,7 @@ NapiHandleScopeImpl::Slot* NapiHandleScopeImpl::reserveSlot() return &m_storage.last(); } -NapiHandleScopeImpl* NapiHandleScope::push(Zig::GlobalObject* globalObject, bool escapable) +NapiHandleScopeImpl* NapiHandleScope::open(Zig::GlobalObject* globalObject, bool escapable) { auto& vm = globalObject->vm(); // Do not create a new handle scope while a finalizer is in progress @@ -101,8 +101,12 @@ NapiHandleScopeImpl* NapiHandleScope::push(Zig::GlobalObject* globalObject, bool return impl; } -void NapiHandleScope::pop(Zig::GlobalObject* globalObject, NapiHandleScopeImpl* current) +void NapiHandleScope::close(Zig::GlobalObject* globalObject, NapiHandleScopeImpl* current) { + // napi handle scopes may be null pointers if created inside a finalizer + if (!current) { + return; + } RELEASE_ASSERT_WITH_MESSAGE(current == globalObject->m_currentNapiHandleScopeImpl.get(), "Unbalanced napi_handle_scope opens and closes"); if (auto* parent = current->parent()) { @@ -114,23 +118,23 @@ void NapiHandleScope::pop(Zig::GlobalObject* globalObject, NapiHandleScopeImpl* NapiHandleScope::NapiHandleScope(Zig::GlobalObject* globalObject) : m_globalObject(globalObject) - , m_impl(NapiHandleScope::push(globalObject, false)) + , m_impl(NapiHandleScope::open(globalObject, false)) { } NapiHandleScope::~NapiHandleScope() { - NapiHandleScope::pop(m_globalObject, m_impl); + NapiHandleScope::close(m_globalObject, m_impl); } -extern "C" NapiHandleScopeImpl* NapiHandleScope__push(Zig::GlobalObject* globalObject, bool escapable) +extern "C" NapiHandleScopeImpl* NapiHandleScope__open(Zig::GlobalObject* globalObject, bool escapable) { - return NapiHandleScope::push(globalObject, escapable); + return NapiHandleScope::open(globalObject, escapable); } -extern "C" void NapiHandleScope__pop(Zig::GlobalObject* globalObject, NapiHandleScopeImpl* current) +extern "C" void NapiHandleScope__close(Zig::GlobalObject* globalObject, NapiHandleScopeImpl* current) { - return NapiHandleScope::pop(globalObject, current); + return NapiHandleScope::close(globalObject, current); } extern "C" void NapiHandleScope__append(Zig::GlobalObject* globalObject, JSC::EncodedJSValue value) diff --git a/src/bun.js/bindings/napi_handle_scope.h b/src/bun.js/bindings/napi_handle_scope.h index 891dbbb9de..851d5006a5 100644 --- a/src/bun.js/bindings/napi_handle_scope.h +++ b/src/bun.js/bindings/napi_handle_scope.h @@ -61,18 +61,18 @@ private: NapiHandleScopeImpl(JSC::VM& vm, JSC::Structure* structure, NapiHandleScopeImpl* parent, bool escapable); }; -// Wrapper class used to push a new handle scope and pop it when this instance goes out of scope +// Wrapper class used to open a new handle scope and close it when this instance goes out of scope class NapiHandleScope { public: NapiHandleScope(Zig::GlobalObject* globalObject); ~NapiHandleScope(); // Create a new handle scope in the given environment - static NapiHandleScopeImpl* push(Zig::GlobalObject* globalObject, bool escapable); + static NapiHandleScopeImpl* open(Zig::GlobalObject* globalObject, bool escapable); - // Pop the most recently created handle scope in the given environment and restore the old one. + // Closes the most recently created handle scope in the given environment and restores the old one. // Asserts that `current` is the active handle scope. - static void pop(Zig::GlobalObject* globalObject, NapiHandleScopeImpl* current); + static void close(Zig::GlobalObject* globalObject, NapiHandleScopeImpl* current); private: NapiHandleScopeImpl* m_impl; @@ -80,11 +80,11 @@ private: }; // Create a new handle scope in the given environment -extern "C" NapiHandleScopeImpl* NapiHandleScope__push(Zig::GlobalObject* globalObject, bool escapable); +extern "C" NapiHandleScopeImpl* NapiHandleScope__open(Zig::GlobalObject* globalObject, bool escapable); // Pop the most recently created handle scope in the given environment and restore the old one. // Asserts that `current` is the active handle scope. -extern "C" void NapiHandleScope__pop(Zig::GlobalObject* globalObject, NapiHandleScopeImpl* current); +extern "C" void NapiHandleScope__close(Zig::GlobalObject* globalObject, NapiHandleScopeImpl* current); // Store a value in the active handle scope in the given environment extern "C" void NapiHandleScope__append(Zig::GlobalObject* globalObject, JSC::EncodedJSValue value); diff --git a/src/napi/napi.zig b/src/napi/napi.zig index dd2a34d804..d1b45a06e0 100644 --- a/src/napi/napi.zig +++ b/src/napi/napi.zig @@ -65,23 +65,33 @@ pub const Ref = opaque { extern fn napi_set_ref(ref: *Ref, value: JSC.JSValue) void; }; pub const NapiHandleScope = opaque { - pub extern fn NapiHandleScope__push(globalObject: *JSC.JSGlobalObject, escapable: bool) ?*NapiHandleScope; - pub extern fn NapiHandleScope__pop(globalObject: *JSC.JSGlobalObject, current: ?*NapiHandleScope) void; + pub extern fn NapiHandleScope__open(globalObject: *JSC.JSGlobalObject, escapable: bool) ?*NapiHandleScope; + pub extern fn NapiHandleScope__close(globalObject: *JSC.JSGlobalObject, current: ?*NapiHandleScope) void; extern fn NapiHandleScope__append(globalObject: *JSC.JSGlobalObject, value: JSC.JSValueReprInt) void; extern fn NapiHandleScope__escape(handleScope: *NapiHandleScope, value: JSC.JSValueReprInt) bool; - pub fn push(env: napi_env, escapable: bool) ?*NapiHandleScope { - return NapiHandleScope__push(env, escapable); + /// Create a new handle scope in the given environment, or return null if creating one now is + /// unsafe (i.e. inside a finalizer) + pub fn open(env: napi_env, escapable: bool) ?*NapiHandleScope { + return NapiHandleScope__open(env, escapable); } - pub fn pop(self: ?*NapiHandleScope, env: napi_env) void { - NapiHandleScope__pop(env, self); + /// Closes the given handle scope, releasing all values inside it, if it is safe to do so. + /// Asserts that self is the current handle scope in env. + pub fn close(self: ?*NapiHandleScope, env: napi_env) void { + NapiHandleScope__close(env, self); } + /// Place a value in the handle scope. Must be done while returning any JS value into NAPI + /// callbacks, as the value must remain alive as long as the handle scope is active, even if the + /// native module doesn't keep it visible on the stack. pub fn append(env: napi_env, value: JSC.JSValue) void { NapiHandleScope__append(env, @intFromEnum(value)); } + /// Move a value from the current handle scope (which must be escapable) to the reserved escape + /// slot in the parent handle scope, allowing that value to outlive the current handle scope. + /// Returns an error if escape() has already been called on this handle scope. pub fn escape(self: *NapiHandleScope, value: JSC.JSValue) error{EscapeCalledTwice}!void { if (!NapiHandleScope__escape(self, @intFromEnum(value))) { return error.EscapeCalledTwice; @@ -749,14 +759,14 @@ pub export fn napi_open_handle_scope(env: napi_env, result_: ?*napi_handle_scope const result = result_ orelse { return invalidArg(); }; - result.* = NapiHandleScope.push(env, false); + result.* = NapiHandleScope.open(env, false); return .ok; } pub export fn napi_close_handle_scope(env: napi_env, handle_scope: napi_handle_scope) napi_status { log("napi_close_handle_scope", .{}); if (handle_scope) |scope| { - scope.pop(env); + scope.close(env); } return .ok; @@ -830,13 +840,13 @@ pub export fn napi_open_escapable_handle_scope(env: napi_env, result_: ?*napi_es const result = result_ orelse { return invalidArg(); }; - result.* = NapiHandleScope.push(env, true); + result.* = NapiHandleScope.open(env, true); return .ok; } pub export fn napi_close_escapable_handle_scope(env: napi_env, scope: napi_escapable_handle_scope) napi_status { log("napi_close_escapable_handle_scope", .{}); if (scope) |s| { - s.pop(env); + s.close(env); } return .ok; } @@ -1203,8 +1213,8 @@ pub const napi_async_work = struct { } pub fn runFromJS(this: *napi_async_work) void { - const handle_scope = NapiHandleScope.push(this.global, false); - defer if (handle_scope) |scope| scope.pop(this.global); + const handle_scope = NapiHandleScope.open(this.global, false); + defer if (handle_scope) |scope| scope.close(this.global); this.complete.?( this.global, if (this.status.load(.seq_cst) == @intFromEnum(Status.cancelled)) @@ -1583,8 +1593,8 @@ pub const ThreadSafeFunction = struct { log("call() {}", .{str}); } - const handle_scope = NapiHandleScope.push(globalObject, false); - defer if (handle_scope) |scope| scope.pop(globalObject); + const handle_scope = NapiHandleScope.open(globalObject, false); + defer if (handle_scope) |scope| scope.close(globalObject); cb.napi_threadsafe_function_call_js(globalObject, napi_value.create(globalObject, cb.js), this.ctx, task); }, }