Skip to content

Commit ce5f9ab

Browse files
committed
node-api: disambiguate napi_add_finalizer
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.
1 parent 1287530 commit ce5f9ab

File tree

3 files changed

+52
-70
lines changed

3 files changed

+52
-70
lines changed

doc/api/n-api.md

Lines changed: 10 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2382,12 +2382,7 @@ is used to pass external data through JavaScript code, so it can be retrieved
23822382
later by native code using [`napi_get_value_external`][].
23832383

23842384
The API adds a `napi_finalize` callback which will be called when the JavaScript
2385-
object just created is ready for garbage collection. It is similar to
2386-
`napi_wrap()` except that:
2387-
2388-
* the native data cannot be retrieved later using `napi_unwrap()`,
2389-
* nor can it be removed later using `napi_remove_wrap()`, and
2390-
* the object created by the API can be used with `napi_wrap()`.
2385+
object just created has been garbage collected.
23912386

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

24432438
The API adds a `napi_finalize` callback which will be called when the JavaScript
2444-
object just created is ready for garbage collection. It is similar to
2445-
`napi_wrap()` except that:
2446-
2447-
* the native data cannot be retrieved later using `napi_unwrap()`,
2448-
* nor can it be removed later using `napi_remove_wrap()`, and
2449-
* the object created by the API can be used with `napi_wrap()`.
2439+
object just created has been garbage collected.
24502440

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

24992489
The API adds a `napi_finalize` callback which will be called when the JavaScript
2500-
object just created is ready for garbage collection. It is similar to
2501-
`napi_wrap()` except that:
2502-
2503-
* the native data cannot be retrieved later using `napi_unwrap()`,
2504-
* nor can it be removed later using `napi_remove_wrap()`, and
2505-
* the object created by the API can be used with `napi_wrap()`.
2490+
object just created has been garbage collected.
25062491

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

@@ -5141,7 +5126,7 @@ napi_status napi_wrap(napi_env env,
51415126
* `[in] native_object`: The native instance that will be wrapped in the
51425127
JavaScript object.
51435128
* `[in] finalize_cb`: Optional native callback that can be used to free the
5144-
native instance when the JavaScript object is ready for garbage-collection.
5129+
native instance when the JavaScript object has been garbage-collected.
51455130
[`napi_finalize`][] provides more details.
51465131
* `[in] finalize_hint`: Optional contextual hint that is passed to the
51475132
finalize callback.
@@ -5303,7 +5288,7 @@ napiVersion: 5
53035288
```c
53045289
napi_status napi_add_finalizer(napi_env env,
53055290
napi_value js_object,
5306-
void* native_object,
5291+
void* finalize_data,
53075292
napi_finalize finalize_cb,
53085293
void* finalize_hint,
53095294
napi_ref* result);
@@ -5312,10 +5297,9 @@ napi_status napi_add_finalizer(napi_env env,
53125297
* `[in] env`: The environment that the API is invoked under.
53135298
* `[in] js_object`: The JavaScript object to which the native data will be
53145299
attached.
5315-
* `[in] native_object`: The native data that will be attached to the JavaScript
5316-
object.
5300+
* `[in] finalize_data`: Optional data to be passed to `finalize_cb`.
53175301
* `[in] finalize_cb`: Native callback that will be used to free the
5318-
native data when the JavaScript object is ready for garbage-collection.
5302+
native data when the JavaScript object has been garbage-collected.
53195303
[`napi_finalize`][] provides more details.
53205304
* `[in] finalize_hint`: Optional contextual hint that is passed to the
53215305
finalize callback.
@@ -5324,14 +5308,9 @@ napi_status napi_add_finalizer(napi_env env,
53245308
Returns `napi_ok` if the API succeeded.
53255309

53265310
Adds a `napi_finalize` callback which will be called when the JavaScript object
5327-
in `js_object` is ready for garbage collection. This API is similar to
5328-
`napi_wrap()` except that:
5329-
5330-
* the native data cannot be retrieved later using `napi_unwrap()`,
5331-
* nor can it be removed later using `napi_remove_wrap()`, and
5332-
* the API can be called multiple times with different data items in order to
5333-
attach each of them to the JavaScript object, and
5334-
* the object manipulated by the API can be used with `napi_wrap()`.
5311+
in `js_object` has been garbage-collected.
5312+
5313+
This API can be called multiple times on a single JavaScript object.
53355314

53365315
_Caution_: The optional returned reference (if obtained) should be deleted via
53375316
[`napi_delete_reference`][] ONLY in response to the finalize callback

src/js_native_api.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_get_date_value(napi_env env,
492492
// Add finalizer for pointer
493493
NAPI_EXTERN napi_status NAPI_CDECL napi_add_finalizer(napi_env env,
494494
napi_value js_object,
495-
void* native_object,
495+
void* finalize_data,
496496
napi_finalize finalize_cb,
497497
void* finalize_hint,
498498
napi_ref* result);

src/js_native_api_v8.cc

Lines changed: 41 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -392,9 +392,6 @@ class FunctionCallbackWrapper : public CallbackWrapperBase {
392392
}
393393
};
394394

395-
enum WrapType { retrievable, anonymous };
396-
397-
template <WrapType wrap_type>
398395
inline napi_status Wrap(napi_env env,
399396
napi_value js_object,
400397
void* native_object,
@@ -410,47 +407,36 @@ inline napi_status Wrap(napi_env env,
410407
RETURN_STATUS_IF_FALSE(env, value->IsObject(), napi_invalid_arg);
411408
v8::Local<v8::Object> obj = value.As<v8::Object>();
412409

413-
if (wrap_type == retrievable) {
414-
// If we've already wrapped this object, we error out.
415-
RETURN_STATUS_IF_FALSE(
416-
env,
417-
!obj->HasPrivate(context, NAPI_PRIVATE_KEY(context, wrapper))
418-
.FromJust(),
419-
napi_invalid_arg);
420-
} else if (wrap_type == anonymous) {
421-
// If no finalize callback is provided, we error out.
422-
CHECK_ARG(env, finalize_cb);
423-
}
410+
// If we've already wrapped this object, we error out.
411+
RETURN_STATUS_IF_FALSE(
412+
env,
413+
!obj->HasPrivate(context, NAPI_PRIVATE_KEY(context, wrapper)).FromJust(),
414+
napi_invalid_arg);
424415

425-
v8impl::Reference* reference = nullptr;
416+
// Create a self-deleting reference if the optional out-param result is not
417+
// set.
418+
bool delete_self = true;
426419
if (result != nullptr) {
427420
// The returned reference should be deleted via napi_delete_reference()
428421
// ONLY in response to the finalize callback invocation. (If it is deleted
429422
// before then, then the finalize callback will never be invoked.)
430423
// Therefore a finalize callback is required when returning a reference.
431424
CHECK_ARG(env, finalize_cb);
432-
reference = v8impl::Reference::New(
433-
env, obj, 0, false, finalize_cb, native_object, finalize_hint);
425+
delete_self = false;
426+
}
427+
428+
v8impl::Reference* reference = v8impl::Reference::New(
429+
env, obj, 0, delete_self, finalize_cb, native_object, finalize_hint);
430+
431+
if (result != nullptr) {
434432
*result = reinterpret_cast<napi_ref>(reference);
435-
} else {
436-
// Create a self-deleting reference.
437-
reference = v8impl::Reference::New(
438-
env,
439-
obj,
440-
0,
441-
true,
442-
finalize_cb,
443-
native_object,
444-
finalize_cb == nullptr ? nullptr : finalize_hint);
445-
}
446-
447-
if (wrap_type == retrievable) {
448-
CHECK(obj->SetPrivate(context,
449-
NAPI_PRIVATE_KEY(context, wrapper),
450-
v8::External::New(env->isolate, reference))
451-
.FromJust());
452433
}
453434

435+
CHECK(obj->SetPrivate(context,
436+
NAPI_PRIVATE_KEY(context, wrapper),
437+
v8::External::New(env->isolate, reference))
438+
.FromJust());
439+
454440
return GET_RETURN_STATUS(env);
455441
}
456442

@@ -2360,7 +2346,7 @@ napi_status NAPI_CDECL napi_wrap(napi_env env,
23602346
napi_finalize finalize_cb,
23612347
void* finalize_hint,
23622348
napi_ref* result) {
2363-
return v8impl::Wrap<v8impl::retrievable>(
2349+
return v8impl::Wrap(
23642350
env, js_object, native_object, finalize_cb, finalize_hint, result);
23652351
}
23662352

@@ -3176,12 +3162,29 @@ napi_status NAPI_CDECL napi_run_script(napi_env env,
31763162

31773163
napi_status NAPI_CDECL napi_add_finalizer(napi_env env,
31783164
napi_value js_object,
3179-
void* native_object,
3165+
void* finalize_data,
31803166
napi_finalize finalize_cb,
31813167
void* finalize_hint,
31823168
napi_ref* result) {
3183-
return v8impl::Wrap<v8impl::anonymous>(
3184-
env, js_object, native_object, finalize_cb, finalize_hint, result);
3169+
// Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw
3170+
// JS exceptions.
3171+
CHECK_ENV(env);
3172+
CHECK_ARG(env, js_object);
3173+
CHECK_ARG(env, finalize_cb);
3174+
3175+
v8::Local<v8::Value> v8_value = v8impl::V8LocalValueFromJsValue(js_object);
3176+
RETURN_STATUS_IF_FALSE(env, v8_value->IsObject(), napi_invalid_arg);
3177+
3178+
// Create a self-deleting reference if the optional out-param result is not
3179+
// set.
3180+
bool delete_self = result == nullptr;
3181+
v8impl::Reference* reference = v8impl::Reference::New(
3182+
env, v8_value, 0, delete_self, finalize_cb, finalize_data, finalize_hint);
3183+
3184+
if (result != nullptr) {
3185+
*result = reinterpret_cast<napi_ref>(reference);
3186+
}
3187+
return napi_clear_last_error(env);
31853188
}
31863189

31873190
napi_status NAPI_CDECL napi_adjust_external_memory(napi_env env,

0 commit comments

Comments
 (0)