disambiguate napi_add_finalizer (nodejs/node#45401)

toyobayashi · Jan 21, 2023 · adbc3d0 · adbc3d0
1 parent 25748cc
commit adbc3d0
Show file tree

Hide file tree

Showing 6 changed files with 64 additions and 36 deletions.
diff --git 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
@@ -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
@@ -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<any>} {
+  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_ref>): 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_ref>): 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
@@ -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
@@ -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
@@ -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_ref>): 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_ref>): 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'])