Skip to content

Commit 0b985bf

Browse files
toyobayashitoyobayashi
committed
refactor: generalize finalizer second pass callback (nodejs/node#44141)
1 parent 6231ddf commit 0b985bf

17 files changed

Lines changed: 322 additions & 147 deletions

‎packages/emnapi/src/emnapi.c‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,8 @@ napi_status napi_cancel_async_work(napi_env env, napi_async_work work) {
363363

364364
#if NAPI_VERSION >= 4 && defined(__EMSCRIPTEN_PTHREADS__)
365365

366+
extern void _emnapi_call_finalizer(napi_env env, napi_finalize cb, void* data, void* hint);
367+
366368
static const unsigned char kDispatchIdle = 0;
367369
static const unsigned char kDispatchRunning = 1 << 0;
368370
static const unsigned char kDispatchPending = 1 << 1;
@@ -538,16 +540,11 @@ static void _emnapi_tsfn_empty_queue_and_delete(napi_threadsafe_function func) {
538540
_emnapi_tsfn_destroy(func);
539541
}
540542

541-
static void _emnapi_tsfn_call_finalize_cb(napi_env env, void* args) {
542-
napi_threadsafe_function func = (napi_threadsafe_function) args;
543-
func->finalize_cb(env, func->finalize_data, func->context);
544-
}
545-
546543
static void _emnapi_tsfn_finalize(napi_threadsafe_function func) {
547544
napi_handle_scope scope;
548545
napi_open_handle_scope(func->env, &scope);
549546
if (func->finalize_cb) {
550-
_emnapi_call_into_module(func->env, _emnapi_tsfn_call_finalize_cb, func);
547+
_emnapi_call_finalizer(func->env, func->finalize_cb, func->finalize_data, func->context);
551548
}
552549
_emnapi_tsfn_empty_queue_and_delete(func);
553550
napi_close_handle_scope(func->env, scope);

‎packages/emnapi/src/env.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
function napi_set_instance_data (env: napi_env, data: void_p, finalize_cb: napi_finalize, finalize_hint: void_p): napi_status {
22
return emnapiCtx.checkEnv(env, (envObject) => {
33
if (envObject.instanceData) {
4-
emnapiRt.RefBase.doDelete(envObject.instanceData)
4+
envObject.instanceData.dispose()
55
}
6-
envObject.instanceData = new emnapiRt.RefBase(envObject, 0, true, finalize_cb, data, finalize_hint)
6+
envObject.instanceData = new emnapiRt.RefBase(envObject, 0, emnapiRt.Ownership.kRuntime, finalize_cb, data, finalize_hint)
77
return envObject.clearLastError()
88
})
99
}

‎packages/emnapi/src/internal.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,12 +158,12 @@ function $emnapiWrap (type: WrapType, env: napi_env, js_object: napi_value, nati
158158
let reference: emnapi.Reference
159159
if (result) {
160160
if (!finalize_cb) return envObject.setLastError(napi_status.napi_invalid_arg)
161-
reference = emnapiRt.Reference.create(envObject, value.id, 0, false, finalize_cb, native_object, finalize_hint)
161+
reference = emnapiRt.Reference.create(envObject, value.id, 0, emnapiRt.Ownership.kUserland, finalize_cb, native_object, finalize_hint)
162162
$from64('result')
163163

164164
$makeSetValue('result', 0, 'reference.id', '*')
165165
} else {
166-
reference = emnapiRt.Reference.create(envObject, value.id, 0, true, finalize_cb, native_object, !finalize_cb ? finalize_cb : finalize_hint)
166+
reference = emnapiRt.Reference.create(envObject, value.id, 0, emnapiRt.Ownership.kRuntime, finalize_cb, native_object, !finalize_cb ? finalize_cb : finalize_hint)
167167
}
168168

169169
if (type === WrapType.retrievable) {
@@ -199,7 +199,12 @@ function $emnapiUnwrap (env: napi_env, js_object: napi_value, result: void_pp, a
199199
}
200200
if (action === UnwrapAction.RemoveWrap) {
201201
value.wrapped = 0
202-
emnapiRt.Reference.doDelete(ref)
202+
if (ref.ownership() === emnapiRt.Ownership.kUserland) {
203+
// When the wrap is been removed, the finalizer should be reset.
204+
ref.resetFinalizer()
205+
} else {
206+
ref.dispose()
207+
}
203208
}
204209
return envObject.getReturnStatus()
205210
})

‎packages/emnapi/src/life.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ function napi_create_reference (
9090
}
9191
// @ts-expect-error
9292
// eslint-disable-next-line @typescript-eslint/no-unused-vars
93-
const ref = emnapiRt.Reference.create(envObject, handle.id, initial_refcount >>> 0, false)
93+
const ref = emnapiRt.Reference.create(envObject, handle.id, initial_refcount >>> 0, emnapiRt.Ownership.kUserland)
9494
$from64('result')
9595
$makeSetValue('result', 0, 'ref.id', '*')
9696
return envObject.clearLastError()
@@ -104,7 +104,7 @@ function napi_delete_reference (
104104
): napi_status {
105105
return emnapiCtx.checkEnv(env, (envObject) => {
106106
return emnapiCtx.checkArgs(envObject, [ref], () => {
107-
emnapiRt.Reference.doDelete(emnapiCtx.refStore.get(ref)!)
107+
emnapiCtx.refStore.get(ref)!.dispose()
108108
return envObject.clearLastError()
109109
})
110110
})

‎packages/emnapi/src/simple-async-operation.ts‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,23 +6,12 @@
66
// declare function __emnapi_async_send_js (callback: number, data: number): void
77
declare function __emnapi_set_immediate (callback: number, data: number): void
88
declare function __emnapi_next_tick (callback: number, data: number): void
9-
declare function emnapiSetImmediate (callback: () => any): any
109

1110
mergeInto(LibraryManager.library, {
12-
$emnapiSetImmediate: 'typeof setImmediate === "function" ? setImmediate : function (f) {' +
13-
'var channel = new MessageChannel();' +
14-
'channel.port1.onmessage = function () {' +
15-
'channel.port1.onmessage = null;' +
16-
'channel = undefined;' +
17-
'f();' +
18-
'};' +
19-
'channel.port2.postMessage(null);' +
20-
'};',
21-
2211
_emnapi_set_immediate__sig: 'vpp',
23-
_emnapi_set_immediate__deps: ['$emnapiGetDynamicCalls', '$emnapiSetImmediate'],
12+
_emnapi_set_immediate__deps: ['$emnapiGetDynamicCalls', '$emnapiInit', '$emnapiRt'],
2413
_emnapi_set_immediate: function (callback: number, data: number): void {
25-
emnapiSetImmediate(() => {
14+
emnapiRt._setImmediate(() => {
2615
$from64('callback')
2716
emnapiGetDynamicCalls.call_vp(callback, data)
2817
})
@@ -99,3 +88,20 @@ mergeInto(LibraryManager.library, {
9988
'return r;' +
10089
'};'
10190
})
91+
92+
function _emnapi_call_into_module (env: napi_env, callback: number, data: number): void {
93+
const envObject = emnapiCtx.envStore.get(env)!
94+
const scope = emnapiCtx.openScope(envObject, emnapiRt.HandleScope)
95+
try {
96+
envObject.callIntoModule((_envObject) => {
97+
$from64('callback')
98+
emnapiGetDynamicCalls.call_vpp(callback, env, data)
99+
})
100+
} catch (err) {
101+
emnapiCtx.closeScope(envObject, scope)
102+
throw err
103+
}
104+
emnapiCtx.closeScope(envObject, scope)
105+
}
106+
107+
emnapiImplement('_emnapi_call_into_module', 'vppp', _emnapi_call_into_module, ['$emnapiGetDynamicCalls'])

‎packages/emnapi/src/thread-safe-function.ts‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,7 @@
1-
function _emnapi_call_into_module (env: napi_env, callback: number, data: number): void {
1+
function _emnapi_call_finalizer (env: napi_env, callback: number, data: number, hint: number): void {
22
const envObject = emnapiCtx.envStore.get(env)!
3-
const scope = emnapiCtx.openScope(envObject, emnapiRt.HandleScope)
4-
try {
5-
envObject.callIntoModule((_envObject) => {
6-
$from64('callback')
7-
emnapiGetDynamicCalls.call_vpp(callback, env, data)
8-
})
9-
} catch (err) {
10-
emnapiCtx.closeScope(envObject, scope)
11-
throw err
12-
}
13-
emnapiCtx.closeScope(envObject, scope)
3+
$from64('callback')
4+
envObject.callFinalizer(callback, data, hint)
145
}
156

167
function _emnapi_tsfn_dispatch_one_js (env: number, ref: number, call_js_cb: number, context: number, data: number): void {
@@ -31,5 +22,5 @@ function _emnapi_tsfn_dispatch_one_js (env: number, ref: number, call_js_cb: num
3122
emnapiCtx.closeScope(envObject, scope)
3223
}
3324

34-
emnapiImplement('_emnapi_call_into_module', 'vpp', _emnapi_call_into_module, ['$emnapiGetDynamicCalls'])
25+
emnapiImplement('_emnapi_call_finalizer', 'vpppp', _emnapi_call_finalizer)
3526
emnapiImplement('_emnapi_tsfn_dispatch_one_js', 'vppppp', _emnapi_tsfn_dispatch_one_js, ['$emnapiGetDynamicCalls'])

‎packages/emnapi/src/value/create.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ function napi_create_external (env: napi_env, data: void_p, finalize_cb: napi_fi
6767
const externalHandle = emnapiRt.ExternalHandle.createExternal(envObject, data)
6868
emnapiCtx.getCurrentScope()!.addHandle(externalHandle)
6969
if (finalize_cb) {
70-
emnapiRt.Reference.create(envObject, externalHandle.id, 0, true, finalize_cb, data, finalize_hint)
70+
emnapiRt.Reference.create(envObject, externalHandle.id, 0, emnapiRt.Ownership.kRuntime, finalize_cb, data, finalize_hint)
7171
}
7272
$from64('result')
7373
$makeSetValue('result', 0, 'externalHandle.id', '*')

‎packages/runtime/src/Finalizer.ts‎

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,25 @@
11
import type { Env } from './env'
22

3-
/** @internal */
4-
export enum EnvReferenceMode {
5-
kNoEnvReference,
6-
kKeepEnvReference
7-
}
8-
93
/** @internal */
104
export class Finalizer {
11-
protected _hasEnvReference: boolean
12-
protected _finalizeRan: boolean
13-
145
public constructor (
156
protected envObject: Env,
167
protected _finalizeCallback: napi_finalize = 0,
178
protected _finalizeData: void_p = 0,
18-
protected _finalizeHint: void_p = 0,
19-
refmode: EnvReferenceMode = EnvReferenceMode.kNoEnvReference
20-
) {
21-
this._finalizeRan = false
22-
this._hasEnvReference = refmode === EnvReferenceMode.kKeepEnvReference
23-
if (this._hasEnvReference) {
24-
envObject.ref()
25-
}
9+
protected _finalizeHint: void_p = 0
10+
) {}
11+
12+
public callback (): napi_finalize { return this._finalizeCallback }
13+
public data (): void_p { return this._finalizeData }
14+
public hint (): void_p { return this._finalizeHint }
15+
16+
public resetFinalizer (): void {
17+
this._finalizeCallback = 0
18+
this._finalizeData = 0
19+
this._finalizeHint = 0
2620
}
2721

2822
public dispose (): void {
29-
if (this._hasEnvReference) {
30-
this.envObject.unref()
31-
}
3223
this.envObject = undefined!
3324
}
3425
}

‎packages/runtime/src/Global.ts‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import { supportFinalizer } from './util'
2+
3+
/** @public */
4+
export class Global<T extends object> {
5+
private _ref: T | WeakRef<T> | null
6+
private _param: any
7+
private _callback: ((param: any) => void) | undefined
8+
9+
private static readonly _registry = supportFinalizer
10+
? new FinalizationRegistry((value: Global<any>) => {
11+
value._ref = null
12+
if (typeof value._callback === 'function') {
13+
value._callback(value._param)
14+
}
15+
})
16+
: undefined!
17+
18+
constructor (value: T) {
19+
this._ref = value
20+
}
21+
22+
setWeak<P> (param: any, callback: (param: P) => void): void {
23+
if (this._ref === null) return
24+
if (!supportFinalizer) return
25+
if (this._ref instanceof WeakRef) return
26+
this._param = param
27+
this._callback = callback
28+
Global._registry.register(this._ref, this, this)
29+
this._ref = new WeakRef<T>(this._ref)
30+
}
31+
32+
clearWeak (): void {
33+
if (this._ref === null) return
34+
if (!supportFinalizer) return
35+
if (this._ref instanceof WeakRef) {
36+
try {
37+
Global._registry.unregister(this)
38+
} catch (_) {}
39+
this._param = undefined
40+
this._callback = undefined
41+
this._ref = this._ref.deref() as T
42+
}
43+
}
44+
45+
reset (other?: T | WeakRef<T>): void {
46+
if (supportFinalizer) {
47+
try {
48+
Global._registry.unregister(this)
49+
} catch (_) {}
50+
}
51+
this._param = undefined
52+
this._callback = undefined
53+
if (other) {
54+
this._ref = other
55+
} else {
56+
this._ref = null
57+
}
58+
}
59+
60+
isEmpty (): boolean {
61+
return this._ref === null
62+
}
63+
64+
deref (): T | undefined {
65+
if (!supportFinalizer) {
66+
return (this._ref as T | null) ?? undefined
67+
}
68+
69+
if (this._ref === null) return undefined
70+
71+
if (this._ref instanceof WeakRef) {
72+
return this._ref.deref()
73+
}
74+
75+
return this._ref
76+
}
77+
}

‎packages/runtime/src/RefBase.ts‎

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
import type { Env } from './env'
22
import { Finalizer } from './Finalizer'
33
import { RefTracker } from './RefTracker'
4-
import { supportFinalizer } from './util'
54

65
/** @internal */
76
export interface RefBase extends Finalizer, RefTracker {}
87

8+
/** @public */
9+
export enum Ownership {
10+
kRuntime,
11+
kUserland
12+
}
13+
914
/** @internal */
1015
export class RefBase extends Finalizer {
1116
public static finalizeAll (list: RefTracker): void {
@@ -21,12 +26,12 @@ export class RefBase extends Finalizer {
2126
}
2227

2328
private _refcount: uint32_t
24-
private _deleteSelf: boolean
29+
private readonly _ownership: Ownership
2530

2631
constructor (
2732
envObject: Env,
2833
initial_refcount: uint32_t,
29-
delete_self: boolean,
34+
ownership: Ownership,
3035
finalize_callback: napi_finalize,
3136
finalize_data: void_p,
3237
finalize_hint: void_p
@@ -35,13 +40,14 @@ export class RefBase extends Finalizer {
3540
;(this as any)._next = null
3641
;(this as any)._prev = null
3742
this._refcount = initial_refcount
38-
this._deleteSelf = delete_self
43+
this._ownership = ownership
3944

4045
this.link(!finalize_callback ? envObject.reflist : envObject.finalizing_reflist)
4146
}
4247

4348
public dispose (): void {
4449
this.unlink()
50+
this.envObject.dequeueFinalizer(this)
4551
super.dispose()
4652
}
4753

@@ -64,39 +70,34 @@ export class RefBase extends Finalizer {
6470
return this._refcount
6571
}
6672

67-
public static doDelete (reference: RefBase): void {
68-
if ((reference.refCount() !== 0) || (reference._deleteSelf) ||
69-
(reference._finalizeRan) || !supportFinalizer) {
70-
reference.dispose()
71-
} else {
72-
// defer until finalizer runs as
73-
// it may already be queued
74-
reference._deleteSelf = true
75-
}
73+
public ownership (): Ownership {
74+
return this._ownership
7675
}
7776

78-
protected finalize (isEnvTeardown = false): void {
79-
if (isEnvTeardown && this.refCount() > 0) this._refcount = 0
77+
public finalize (): void {
78+
const ownership = this._ownership
79+
// Swap out the field finalize_callback so that it can not be accidentally
80+
// called more than once.
81+
const finalize_callback = this._finalizeCallback
82+
const finalize_data = this._finalizeData
83+
const finalize_hint = this._finalizeHint
84+
this.resetFinalizer()
85+
86+
this.unlink()
8087

8188
let error: any
8289
let caught = false
83-
if (this._finalizeCallback) {
84-
const fini = Number(this._finalizeCallback)
85-
this._finalizeCallback = 0
90+
if (finalize_callback) {
91+
const fini = Number(finalize_callback)
8692
try {
87-
this.envObject.callFinalizer(fini, this._finalizeData, this._finalizeHint)
93+
this.envObject.callFinalizer(fini, finalize_data, finalize_hint)
8894
} catch (err) {
8995
caught = true
9096
error = err
9197
}
9298
}
93-
if (this._deleteSelf || isEnvTeardown) {
94-
RefBase.doDelete(this)
95-
} else {
96-
this._finalizeRan = true
97-
// leak if this is a non-self-delete weak reference
98-
// should call napi_delete_referece manually
99-
// Reference.doDelete(this)
99+
if (ownership === Ownership.kRuntime) {
100+
this.dispose()
100101
}
101102
if (caught) {
102103
throw error

0 commit comments

Comments
 (0)