Skip to content

Commit

Permalink
Fix beforeOnExit being dispatched multiple times
Browse files Browse the repository at this point in the history
  • Loading branch information
heimskr committed Nov 13, 2024
1 parent 7b25ce1 commit a8fa566
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 9 deletions.
12 changes: 8 additions & 4 deletions src/bun.js/bindings/BunProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2792,12 +2792,16 @@ extern "C" void Bun__Process__queueNextTick1(GlobalObject* globalObject, Encoded
}
JSC_DECLARE_HOST_FUNCTION(Bun__Process__queueNextTick1);

extern "C" void Bun__queueFinishNapiFinalizers(JSGlobalObject* jsGlobalObject)
extern "C" bool Bun__queueFinishNapiFinalizers(JSGlobalObject* jsGlobalObject)
{
Zig::GlobalObject* globalObject = jsCast<Zig::GlobalObject*>(jsGlobalObject);
globalObject->queueTask(new EventLoopTask([globalObject](ScriptExecutionContext&) {
globalObject->finishNapiFinalizers();
}));
if (globalObject->hasNapiFinalizers()) {
globalObject->queueTask(new EventLoopTask([globalObject](ScriptExecutionContext&) {
globalObject->finishNapiFinalizers();

This comment has been minimized.

Copy link
@Jarred-Sumner

Jarred-Sumner Nov 13, 2024

Collaborator

context->jsGlobalObject()->finishNapiFinalizers()

}));
return true;
}
return false;
}

JSValue Process::constructNextTickFn(JSC::VM& vm, Zig::GlobalObject* globalObject)
Expand Down
17 changes: 17 additions & 0 deletions src/bun.js/bindings/ZigGlobalObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4339,6 +4339,23 @@ void GlobalObject::finishNapiFinalizers()
}
}

extern "C" bool Bun__isNapiFinalizerQueueEmpty(const JSGlobalObject*);

bool GlobalObject::hasNapiFinalizers() const
{
if (!Bun__isNapiFinalizerQueueEmpty(this)) {
return true;
}

for (const auto& env : m_napiEnvs) {
if (env->hasFinalizers()) {
return true;
}
}

return false;
}

#include "ZigGeneratedClasses+lazyStructureImpl.h"
#include "ZigGlobalObject.lut.h"

Expand Down
1 change: 1 addition & 0 deletions src/bun.js/bindings/ZigGlobalObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,7 @@ class GlobalObject : public Bun::GlobalScope {
napi_env makeNapiEnv(const napi_module&);
napi_env makeNapiEnvForFFI();
void finishNapiFinalizers();
bool hasNapiFinalizers() const;

private:
DOMGuardedObjectSet m_guardedObjects WTF_GUARDED_BY_LOCK(m_gcLock);
Expand Down
5 changes: 5 additions & 0 deletions src/bun.js/bindings/napi.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ struct napi_env__ {
m_finalizers.emplace(callback, hint, data);
}

bool hasFinalizers() const
{
return !m_finalizers.empty();
}

void checkGC()
{
if (UNLIKELY(m_globalObject->vm().heap.mutatorState() == JSC::MutatorState::Sweeping)) {
Expand Down
2 changes: 1 addition & 1 deletion src/bun.js/bindings/napi_external.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace Bun {
NapiExternal::~NapiExternal()
{
ASSERT(m_env);
m_finalizer.call(m_env, m_value);
m_finalizer.call(m_env, m_value, !m_env->mustDeferFinalizers());
}

void NapiExternal::destroy(JSC::JSCell* cell)
Expand Down
10 changes: 6 additions & 4 deletions src/bun.js/javascript.zig
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,11 @@ pub export fn Bun__GlobalObject__hasIPC(global: *JSC.JSGlobalObject) bool {
}

extern fn Bun__Process__queueNextTick1(*JSC.ZigGlobalObject, JSC.JSValue, JSC.JSValue) void;
extern fn Bun__queueFinishNapiFinalizers(?*JSC.JSGlobalObject) callconv(.C) void;
extern fn Bun__queueFinishNapiFinalizers(?*JSC.JSGlobalObject) callconv(.C) bool;

pub export fn Bun__isNapiFinalizerQueueEmpty(globalObject: *JSGlobalObject) callconv(JSC.conv) bool {
return globalObject.bunVM().eventLoop().napi_finalizer_queue.count == 0;
}

pub export fn Bun__Process__send(
globalObject: *JSGlobalObject,
Expand Down Expand Up @@ -1319,9 +1323,7 @@ pub const VirtualMachine = struct {
}

pub fn onBeforeExit(this: *VirtualMachine) void {
Bun__queueFinishNapiFinalizers(this.global);

if (this.eventLoop().napi_finalizer_queue.count > 0) {
if (Bun__queueFinishNapiFinalizers(this.global) or this.eventLoop().napi_finalizer_queue.count > 0) {
// If we have any finalizers queued, we need to run the event loop until the finalizers are done.
// If there are no finalizers remaining, this isn't necessary.
while (this.isEventLoopAlive()) {
Expand Down

0 comments on commit a8fa566

Please sign in to comment.