From adbc3d01a09999b7dab195fc25793d48a095ef35 Mon Sep 17 00:00:00 2001 From: toyobayashi Date: Sat, 21 Jan 2023 19:42:27 +0800 Subject: [PATCH] disambiguate napi_add_finalizer (nodejs/node#45401) --- packages/emnapi/include/js_native_api.h | 2 +- packages/emnapi/src/emnapi.ts | 4 +-- packages/emnapi/src/internal.ts | 46 +++++++++++++++---------- packages/emnapi/src/typings/enum.d.ts | 5 --- packages/emnapi/src/value/create.ts | 4 +-- packages/emnapi/src/wrap.ts | 39 +++++++++++++++++---- 6 files changed, 64 insertions(+), 36 deletions(-) diff --git a/packages/emnapi/include/js_native_api.h b/packages/emnapi/include/js_native_api.h index ed7e1d04..d57f24b4 100644 --- a/packages/emnapi/include/js_native_api.h +++ b/packages/emnapi/include/js_native_api.h @@ -474,7 +474,7 @@ NAPI_EXTERN napi_status napi_get_date_value(napi_env env, // Add finalizer for pointer NAPI_EXTERN napi_status napi_add_finalizer(napi_env env, napi_value js_object, - void* native_object, + void* finalize_data, napi_finalize finalize_cb, void* finalize_hint, napi_ref* result); diff --git a/packages/emnapi/src/emnapi.ts b/packages/emnapi/src/emnapi.ts index 57de0e90..bdbd939e 100644 --- a/packages/emnapi/src/emnapi.ts +++ b/packages/emnapi/src/emnapi.ts @@ -119,7 +119,7 @@ function emnapi_create_memory_view ( const handle = emnapiCtx.addToCurrentScope(typedArray) emnapiExternalMemory.wasmMemoryViewTable.set(typedArray, viewDescriptor) if (finalize_cb) { - const status = emnapiWrap(WrapType.anonymous, env, handle.id, external_data, finalize_cb, finalize_hint, /* NULL */ 0) + const status = _napi_add_finalizer(env, handle.id, external_data, finalize_cb, finalize_hint, /* NULL */ 0) if (status === napi_status.napi_pending_exception) { const err = envObject.tryCatch.extractException() envObject.clearLastError() @@ -293,6 +293,6 @@ emnapiImplement('emnapi_is_support_weakref', 'i', emnapi_is_support_weakref) emnapiImplement('emnapi_is_support_bigint', 'i', emnapi_is_support_bigint) emnapiImplement('emnapi_get_module_object', 'ipp', emnapi_get_module_object) emnapiImplement('emnapi_get_module_property', 'ippp', emnapi_get_module_property) -emnapiImplement('emnapi_create_memory_view', 'ipippppp', emnapi_create_memory_view, ['$emnapiWrap', '$emnapiExternalMemory']) +emnapiImplement('emnapi_create_memory_view', 'ipippppp', emnapi_create_memory_view, ['napi_add_finalizer', '$emnapiExternalMemory']) emnapiImplement('emnapi_sync_memory', 'ipppppi', emnapi_sync_memory, ['$emnapiSyncMemory']) emnapiImplement('emnapi_get_memory_address', 'ipppp', emnapi_get_memory_address, ['$emnapiGetMemoryAddress']) diff --git a/packages/emnapi/src/internal.ts b/packages/emnapi/src/internal.ts index 310257af..97e4ee61 100644 --- a/packages/emnapi/src/internal.ts +++ b/packages/emnapi/src/internal.ts @@ -92,31 +92,40 @@ function _$emnapiDefineProperty (envObject: Env, obj: object, propertyName: stri } } +// eslint-disable-next-line @typescript-eslint/no-unused-vars +declare const emnapiGetHandle: typeof _$emnapiGetHandle +function _$emnapiGetHandle (js_object: napi_value): { status: napi_status; handle?: Handle} { + let handle = emnapiCtx.handleStore.get(js_object)! + if (!(handle.isObject() || handle.isFunction())) { + return { status: napi_status.napi_invalid_arg } + } + + if (typeof emnapiExternalMemory !== 'undefined' && ArrayBuffer.isView(handle.value)) { + if (emnapiExternalMemory.wasmMemoryViewTable.has(handle.value)) { + handle = emnapiCtx.addToCurrentScope(emnapiExternalMemory.wasmMemoryViewTable.get(handle.value)!) + } + } + + return { status: napi_status.napi_ok, handle } +} + // eslint-disable-next-line @typescript-eslint/no-unused-vars declare const emnapiWrap: typeof _$emnapiWrap // @ts-expect-error -function _$emnapiWrap (type: WrapType, env: napi_env, js_object: napi_value, native_object: void_p, finalize_cb: napi_finalize, finalize_hint: void_p, result: Pointer): napi_status { +function _$emnapiWrap (env: napi_env, js_object: napi_value, native_object: void_p, finalize_cb: napi_finalize, finalize_hint: void_p, result: Pointer): napi_status { // eslint-disable-next-line @typescript-eslint/no-unused-vars let referenceId: number $PREAMBLE!(env, (envObject) => { $CHECK_ARG!(envObject, js_object) - let handle = emnapiCtx.handleStore.get(js_object)! - if (!(handle.isObject() || handle.isFunction())) { - return envObject.setLastError(napi_status.napi_invalid_arg) - } - if (typeof emnapiExternalMemory !== 'undefined' && ArrayBuffer.isView(handle.value)) { - if (emnapiExternalMemory.wasmMemoryViewTable.has(handle.value)) { - handle = emnapiCtx.addToCurrentScope(emnapiExternalMemory.wasmMemoryViewTable.get(handle.value)!) - } + const handleResult = emnapiGetHandle(js_object) + if (handleResult.status !== napi_status.napi_ok) { + return envObject.setLastError(handleResult.status) } + const handle = handleResult.handle! - if (type === WrapType.retrievable) { - if (envObject.getObjectBinding(handle.value).wrapped !== 0) { - return envObject.setLastError(napi_status.napi_invalid_arg) - } - } else if (type === WrapType.anonymous) { - if (!finalize_cb) return envObject.setLastError(napi_status.napi_invalid_arg) + if (envObject.getObjectBinding(handle.value).wrapped !== 0) { + return envObject.setLastError(napi_status.napi_invalid_arg) } let reference: Reference @@ -130,9 +139,7 @@ function _$emnapiWrap (type: WrapType, env: napi_env, js_object: napi_value, nat reference = emnapiCtx.createReference(envObject, handle.id, 0, Ownership.kRuntime as any, finalize_cb, native_object, !finalize_cb ? finalize_cb : finalize_hint) } - if (type === WrapType.retrievable) { - envObject.getObjectBinding(handle.value).wrapped = reference.id - } + envObject.getObjectBinding(handle.value).wrapped = reference.id return envObject.getReturnStatus() }) } @@ -178,5 +185,6 @@ function _$emnapiUnwrap (env: napi_env, js_object: napi_value, result: void_pp, emnapiImplement('$emnapiCreateFunction', undefined, _$emnapiCreateFunction, ['$emnapiUtf8ToString']) emnapiImplement('$emnapiDefineProperty', undefined, _$emnapiDefineProperty, ['$emnapiCreateFunction']) -emnapiImplement('$emnapiWrap', undefined, _$emnapiWrap) +emnapiImplement('$emnapiGetHandle', undefined, _$emnapiGetHandle) +emnapiImplement('$emnapiWrap', undefined, _$emnapiWrap, ['$emnapiGetHandle']) emnapiImplement('$emnapiUnwrap', undefined, _$emnapiUnwrap) diff --git a/packages/emnapi/src/typings/enum.d.ts b/packages/emnapi/src/typings/enum.d.ts index 13eade4f..6a25fbae 100644 --- a/packages/emnapi/src/typings/enum.d.ts +++ b/packages/emnapi/src/typings/enum.d.ts @@ -1,8 +1,3 @@ -declare const enum WrapType { - retrievable, - anonymous -} - declare const enum UnwrapAction { KeepWrap, RemoveWrap diff --git a/packages/emnapi/src/value/create.ts b/packages/emnapi/src/value/create.ts index 45dda7a1..04d51917 100644 --- a/packages/emnapi/src/value/create.ts +++ b/packages/emnapi/src/value/create.ts @@ -134,7 +134,7 @@ function napi_create_external_arraybuffer ( } const handle = emnapiCtx.addToCurrentScope(arrayBuffer) if (finalize_cb) { - const status = emnapiWrap(WrapType.anonymous, env, handle.id, external_data, finalize_cb, finalize_hint, /* NULL */ 0) + const status = _napi_add_finalizer(env, handle.id, external_data, finalize_cb, finalize_hint, /* NULL */ 0) if (status === napi_status.napi_pending_exception) { const err = envObject.tryCatch.extractException() envObject.clearLastError() @@ -449,7 +449,7 @@ emnapiImplement('napi_create_buffer', 'ippp', napi_create_buffer, ['$emnapiExter emnapiImplement('napi_create_buffer_copy', 'ippppp', napi_create_buffer_copy, ['$emnapiCreateArrayBuffer']) emnapiImplement('napi_create_date', 'ipdp', napi_create_date) emnapiImplement('napi_create_external', 'ippppp', napi_create_external) -emnapiImplement('napi_create_external_arraybuffer', 'ipppppp', napi_create_external_arraybuffer, ['$emnapiWrap']) +emnapiImplement('napi_create_external_arraybuffer', 'ipppppp', napi_create_external_arraybuffer, ['napi_add_finalizer']) emnapiImplement('napi_create_external_buffer', 'ipppppp', napi_create_external_buffer, ['emnapi_create_memory_view']) emnapiImplement('napi_create_object', 'ipp', napi_create_object) emnapiImplement('napi_create_symbol', 'ippp', napi_create_symbol) diff --git a/packages/emnapi/src/wrap.ts b/packages/emnapi/src/wrap.ts index aade7bae..bc60440c 100644 --- a/packages/emnapi/src/wrap.ts +++ b/packages/emnapi/src/wrap.ts @@ -87,7 +87,7 @@ function napi_wrap (env: napi_env, js_object: napi_value, native_object: void_p, }) } } - return emnapiWrap(WrapType.retrievable, env, js_object, native_object, finalize_cb, finalize_hint, result) + return emnapiWrap(env, js_object, native_object, finalize_cb, finalize_hint, result) } function napi_unwrap (env: napi_env, js_object: napi_value, result: void_pp): napi_status { @@ -164,13 +164,38 @@ function napi_check_object_type_tag (env: napi_env, object: napi_value, type_tag }) } -function napi_add_finalizer (env: napi_env, js_object: napi_value, native_object: void_p, finalize_cb: napi_finalize, finalize_hint: void_p, result: Pointer): napi_status { +// eslint-disable-next-line @typescript-eslint/no-unused-vars +declare const _napi_add_finalizer: typeof napi_add_finalizer +function napi_add_finalizer (env: napi_env, js_object: napi_value, finalize_data: void_p, finalize_cb: napi_finalize, finalize_hint: void_p, result: Pointer): napi_status { + $CHECK_ENV!(env) + const envObject = emnapiCtx.envStore.get(env)! + if (!emnapiCtx.feature.supportFinalizer) { - $PREAMBLE!(env, () => { - throw emnapiCtx.createNotSupportWeakRefError('napi_add_finalizer', 'This API is unavailable') - }) + return envObject.setLastError(napi_status.napi_generic_failure) + } + + $CHECK_ARG!(envObject, js_object) + $CHECK_ARG!(envObject, finalize_cb) + + const handleResult = emnapiGetHandle(js_object) + if (handleResult.status !== napi_status.napi_ok) { + return envObject.setLastError(handleResult.status) } - return emnapiWrap(WrapType.anonymous, env, js_object, native_object, finalize_cb, finalize_hint, result) + const handle = handleResult.handle! + + const ownership: Ownership = !result ? Ownership.kRuntime : Ownership.kUserland + $from64('finalize_data') + $from64('finalize_cb') + $from64('finalize_hint') + const reference = emnapiCtx.createReference(envObject, handle.id, 0, ownership as any, finalize_cb, finalize_data, finalize_hint) + if (result) { + $from64('result') + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const referenceId = reference.id + $makeSetValue('result', 0, 'referenceId', '*') + } + + return envObject.clearLastError() } emnapiImplement('napi_define_class', 'ipppppppp', napi_define_class, ['$emnapiCreateFunction', '$emnapiDefineProperty']) @@ -179,4 +204,4 @@ emnapiImplement('napi_unwrap', 'ippp', napi_unwrap, ['$emnapiUnwrap']) emnapiImplement('napi_remove_wrap', 'ippp', napi_remove_wrap, ['$emnapiUnwrap']) emnapiImplement('napi_type_tag_object', 'ippp', napi_type_tag_object) emnapiImplement('napi_check_object_type_tag', 'ipppp', napi_check_object_type_tag) -emnapiImplement('napi_add_finalizer', 'ipppppp', napi_add_finalizer, ['$emnapiWrap']) +emnapiImplement('napi_add_finalizer', 'ipppppp', napi_add_finalizer, ['$emnapiGetHandle'])