Skip to content

Commit

Permalink
node-api: disambiguate napi_add_finalizer
Browse files Browse the repository at this point in the history 
napi_add_finalizer doesn't support any operations related to
napi_wrap. Remove the ambiguous statements in the doc about
napi_wrap and avoid reusing the v8impl::Wrap call.

PR-URL: #45401
Reviewed-By: Michael Dawson <midawson@redhat.com>
  • Loading branch information
@legendecas @juanarbol
legendecas authored and juanarbol committed Jan 31, 2023
1 parent 86554bf commit 2ab35cf
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 56 deletions.
41 changes: 10 additions & 31 deletions doc/api/n-api.md
View file
Expand Up @@ -2382,12 +2382,7 @@ is used to pass external data through JavaScript code, so it can be retrieved
later by native code using [`napi_get_value_external`][].

The API adds a `napi_finalize` callback which will be called when the JavaScript
object just created is ready for garbage collection. It is similar to
`napi_wrap()` except that:

* the native data cannot be retrieved later using `napi_unwrap()`,
* nor can it be removed later using `napi_remove_wrap()`, and
* the object created by the API can be used with `napi_wrap()`.
object just created has been garbage collected.

The created value is not an object, and therefore does not support additional
properties. It is considered a distinct value type: calling `napi_typeof()` with
Expand Down Expand Up @@ -2441,12 +2436,7 @@ managed. The caller must ensure that the byte buffer remains valid until the
finalize callback is called.

The API adds a `napi_finalize` callback which will be called when the JavaScript
object just created is ready for garbage collection. It is similar to
`napi_wrap()` except that:

* the native data cannot be retrieved later using `napi_unwrap()`,
* nor can it be removed later using `napi_remove_wrap()`, and
* the object created by the API can be used with `napi_wrap()`.
object just created has been garbage collected.

JavaScript `ArrayBuffer`s are described in
[Section 24.1][] of the ECMAScript Language Specification.
Expand Down Expand Up @@ -2497,12 +2487,7 @@ backed by the passed in buffer. While this is still a fully-supported data
structure, in most cases using a `TypedArray` will suffice.

The API adds a `napi_finalize` callback which will be called when the JavaScript
object just created is ready for garbage collection. It is similar to
`napi_wrap()` except that:

* the native data cannot be retrieved later using `napi_unwrap()`,
* nor can it be removed later using `napi_remove_wrap()`, and
* the object created by the API can be used with `napi_wrap()`.
object just created has been garbage collected.

For Node.js >=4 `Buffers` are `Uint8Array`s.

Expand Down Expand Up @@ -5139,7 +5124,7 @@ napi_status napi_wrap(napi_env env,
* `[in] native_object`: The native instance that will be wrapped in the
JavaScript object.
* `[in] finalize_cb`: Optional native callback that can be used to free the
native instance when the JavaScript object is ready for garbage-collection.
native instance when the JavaScript object has been garbage-collected.
[`napi_finalize`][] provides more details.
* `[in] finalize_hint`: Optional contextual hint that is passed to the
finalize callback.
Expand Down Expand Up @@ -5301,7 +5286,7 @@ napiVersion: 5
```c
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);
Expand All @@ -5310,10 +5295,9 @@ napi_status napi_add_finalizer(napi_env env,
* `[in] env`: The environment that the API is invoked under.
* `[in] js_object`: The JavaScript object to which the native data will be
attached.
* `[in] native_object`: The native data that will be attached to the JavaScript
object.
* `[in] finalize_data`: Optional data to be passed to `finalize_cb`.
* `[in] finalize_cb`: Native callback that will be used to free the
native data when the JavaScript object is ready for garbage-collection.
native data when the JavaScript object has been garbage-collected.
[`napi_finalize`][] provides more details.
* `[in] finalize_hint`: Optional contextual hint that is passed to the
finalize callback.
Expand All @@ -5322,14 +5306,9 @@ napi_status napi_add_finalizer(napi_env env,
Returns `napi_ok` if the API succeeded.

Adds a `napi_finalize` callback which will be called when the JavaScript object
in `js_object` is ready for garbage collection. This API is similar to
`napi_wrap()` except that:

* the native data cannot be retrieved later using `napi_unwrap()`,
* nor can it be removed later using `napi_remove_wrap()`, and
* the API can be called multiple times with different data items in order to
attach each of them to the JavaScript object, and
* the object manipulated by the API can be used with `napi_wrap()`.
in `js_object` has been garbage-collected.

This API can be called multiple times on a single JavaScript object.

_Caution_: The optional returned reference (if obtained) should be deleted via
[`napi_delete_reference`][] ONLY in response to the finalize callback
Expand Down
2 changes: 1 addition & 1 deletion src/js_native_api.h
View file
Expand Up @@ -492,7 +492,7 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_get_date_value(napi_env env,
// Add finalizer for pointer
NAPI_EXTERN napi_status NAPI_CDECL 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);
Expand Down
56 changes: 32 additions & 24 deletions src/js_native_api_v8.cc
View file
Expand Up @@ -401,9 +401,6 @@ class FunctionCallbackWrapper : public CallbackWrapperBase {
}
};

enum WrapType { retrievable, anonymous };

template <WrapType wrap_type>
inline napi_status Wrap(napi_env env,
napi_value js_object,
void* native_object,
Expand All @@ -419,17 +416,11 @@ inline napi_status Wrap(napi_env env,
RETURN_STATUS_IF_FALSE(env, value->IsObject(), napi_invalid_arg);
v8::Local<v8::Object> obj = value.As<v8::Object>();

if (wrap_type == retrievable) {
// If we've already wrapped this object, we error out.
RETURN_STATUS_IF_FALSE(
env,
!obj->HasPrivate(context, NAPI_PRIVATE_KEY(context, wrapper))
.FromJust(),
napi_invalid_arg);
} else if (wrap_type == anonymous) {
// If no finalize callback is provided, we error out.
CHECK_ARG(env, finalize_cb);
}
// If we've already wrapped this object, we error out.
RETURN_STATUS_IF_FALSE(
env,
!obj->HasPrivate(context, NAPI_PRIVATE_KEY(context, wrapper)).FromJust(),
napi_invalid_arg);

v8impl::Reference* reference = nullptr;
if (result != nullptr) {
Expand Down Expand Up @@ -458,12 +449,10 @@ inline napi_status Wrap(napi_env env,
finalize_cb == nullptr ? nullptr : finalize_hint);
}

if (wrap_type == retrievable) {
CHECK(obj->SetPrivate(context,
NAPI_PRIVATE_KEY(context, wrapper),
v8::External::New(env->isolate, reference))
.FromJust());
}
CHECK(obj->SetPrivate(context,
NAPI_PRIVATE_KEY(context, wrapper),
v8::External::New(env->isolate, reference))
.FromJust());

return GET_RETURN_STATUS(env);
}
Expand Down Expand Up @@ -2289,7 +2278,7 @@ napi_status NAPI_CDECL napi_wrap(napi_env env,
napi_finalize finalize_cb,
void* finalize_hint,
napi_ref* result) {
return v8impl::Wrap<v8impl::retrievable>(
return v8impl::Wrap(
env, js_object, native_object, finalize_cb, finalize_hint, result);
}

Expand Down Expand Up @@ -3110,12 +3099,31 @@ napi_status NAPI_CDECL napi_run_script(napi_env env,

napi_status NAPI_CDECL 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) {
return v8impl::Wrap<v8impl::anonymous>(
env, js_object, native_object, finalize_cb, finalize_hint, result);
// Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw
// JS exceptions.
CHECK_ENV(env);
CHECK_ARG(env, js_object);
CHECK_ARG(env, finalize_cb);

v8::Local<v8::Value> v8_value = v8impl::V8LocalValueFromJsValue(js_object);
RETURN_STATUS_IF_FALSE(env, v8_value->IsObject(), napi_invalid_arg);

// Create a self-deleting reference if the optional out-param result is not
// set.
v8impl::Ownership ownership = result == nullptr
? v8impl::Ownership::kRuntime
: v8impl::Ownership::kUserland;
v8impl::Reference* reference = v8impl::Reference::New(
env, v8_value, 0, ownership, finalize_cb, finalize_data, finalize_hint);

if (result != nullptr) {
*result = reinterpret_cast<napi_ref>(reference);
}
return napi_clear_last_error(env);
}

napi_status NAPI_CDECL napi_adjust_external_memory(napi_env env,
Expand Down

0 comments on commit 2ab35cf

Please sign in to comment.