diff --git a/benchmark/napi/ref/.gitignore b/benchmark/napi/ref/.gitignore new file mode 100644 index 00000000000000..567609b1234a9b --- /dev/null +++ b/benchmark/napi/ref/.gitignore @@ -0,0 +1 @@ +build/ diff --git a/benchmark/napi/ref/addon.c b/benchmark/napi/ref/addon.c new file mode 100644 index 00000000000000..3fb8de603d3ced --- /dev/null +++ b/benchmark/napi/ref/addon.c @@ -0,0 +1,82 @@ +#include +#define NAPI_EXPERIMENTAL +#include + +#define NAPI_CALL(env, call) \ + do { \ + napi_status status = (call); \ + if (status != napi_ok) { \ + napi_throw_error((env), NULL, #call " failed"); \ + return NULL; \ + } \ + } while (0) + +static napi_value +GetCount(napi_env env, napi_callback_info info) { + napi_value result; + size_t* count; + + NAPI_CALL(env, napi_get_instance_data(env, (void**)&count)); + NAPI_CALL(env, napi_create_uint32(env, *count, &result)); + + return result; +} + +static napi_value +SetCount(napi_env env, napi_callback_info info) { + size_t* count; + + NAPI_CALL(env, napi_get_instance_data(env, (void**)&count)); + + // Set the count to zero irrespective of what is passed into the setter. + *count = 0; + + return NULL; +} + +static void +IncrementCounter(napi_env env, void* data, void* hint) { + size_t* count = data; + (*count) = (*count) + 1; +} + +static napi_value +NewWeak(napi_env env, napi_callback_info info) { + napi_value result; + void* instance_data; + + NAPI_CALL(env, napi_create_object(env, &result)); + NAPI_CALL(env, napi_get_instance_data(env, &instance_data)); + NAPI_CALL(env, napi_add_finalizer(env, + result, + instance_data, + IncrementCounter, + NULL, + NULL)); + + return result; +} + +static void +FreeCount(napi_env env, void* data, void* hint) { + free(data); +} + +/* napi_value */ +NAPI_MODULE_INIT(/* napi_env env, napi_value exports */) { + napi_property_descriptor props[] = { + { "count", NULL, NULL, GetCount, SetCount, NULL, napi_enumerable, NULL }, + { "newWeak", NULL, NewWeak, NULL, NULL, NULL, napi_enumerable, NULL } + }; + + size_t* count = malloc(sizeof(*count)); + *count = 0; + + NAPI_CALL(env, napi_define_properties(env, + exports, + sizeof(props) / sizeof(*props), + props)); + NAPI_CALL(env, napi_set_instance_data(env, count, FreeCount, NULL)); + + return exports; +} diff --git a/benchmark/napi/ref/binding.gyp b/benchmark/napi/ref/binding.gyp new file mode 100644 index 00000000000000..d641e99efd77cf --- /dev/null +++ b/benchmark/napi/ref/binding.gyp @@ -0,0 +1,10 @@ +{ + 'targets': [ + { + 'target_name': 'addon', + 'sources': [ + 'addon.c' + ] + } + ] +} diff --git a/benchmark/napi/ref/index.js b/benchmark/napi/ref/index.js new file mode 100644 index 00000000000000..3a5e1988275eaa --- /dev/null +++ b/benchmark/napi/ref/index.js @@ -0,0 +1,17 @@ +'use strict'; +const common = require('../../common'); +const addon = require(`./build/${common.buildType}/addon`); +const bench = common.createBenchmark(main, { n: [1e7] }); + +function callNewWeak() { + addon.newWeak(); +} + +function main({ n }) { + addon.count = 0; + bench.start(); + while (addon.count < n) { + callNewWeak(); + } + bench.end(n); +} diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index a87af79a890832..eea8adea3769ee 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -186,7 +186,7 @@ inline static napi_status ConcludeDeferred(napi_env env, } // Wrapper around v8impl::Persistent that implements reference counting. -class Reference : private Finalizer { +class Reference : private Finalizer, RefTracker { private: Reference(napi_env env, v8::Local value, @@ -203,6 +203,9 @@ class Reference : private Finalizer { _persistent.SetWeak( this, FinalizeCallback, v8::WeakCallbackType::kParameter); } + Link(finalize_callback == nullptr + ? &env->reflist + : &env->finalizing_reflist); } public: @@ -242,6 +245,7 @@ class Reference : private Finalizer { // the finalizer and _delete_self is set. In this case we // know we need to do the deletion so just do it. static void Delete(Reference* reference) { + reference->Unlink(); if ((reference->RefCount() != 0) || (reference->_delete_self) || (reference->_finalize_ran)) { @@ -286,6 +290,26 @@ class Reference : private Finalizer { } private: + void Finalize(bool is_env_teardown = false) override { + if (_finalize_callback != nullptr) { + _env->CallIntoModuleThrow([&](napi_env env) { + _finalize_callback( + env, + _finalize_data, + _finalize_hint); + }); + } + + // this is safe because if a request to delete the reference + // is made in the finalize_callback it will defer deletion + // to this block and set _delete_self to true + if (_delete_self || is_env_teardown) { + Delete(this); + } else { + _finalize_ran = true; + } + } + // The N-API finalizer callback may make calls into the engine. V8's heap is // not in a consistent state during the weak callback, and therefore it does // not support calls back into it. However, it provides a mechanism for adding @@ -303,25 +327,7 @@ class Reference : private Finalizer { } static void SecondPassCallback(const v8::WeakCallbackInfo& data) { - Reference* reference = data.GetParameter(); - - if (reference->_finalize_callback != nullptr) { - reference->_env->CallIntoModuleThrow([&](napi_env env) { - reference->_finalize_callback( - env, - reference->_finalize_data, - reference->_finalize_hint); - }); - } - - // this is safe because if a request to delete the reference - // is made in the finalize_callback it will defer deletion - // to this block and set _delete_self to true - if (reference->_delete_self) { - Delete(reference); - } else { - reference->_finalize_ran = true; - } + data.GetParameter()->Finalize(); } v8impl::Persistent _persistent; diff --git a/src/js_native_api_v8.h b/src/js_native_api_v8.h index 506e693f821227..2e0a7a1d6add20 100644 --- a/src/js_native_api_v8.h +++ b/src/js_native_api_v8.h @@ -15,6 +15,13 @@ struct napi_env__ { CHECK_EQ(isolate, context->GetIsolate()); } virtual ~napi_env__() { + // First we must finalize those references that have `napi_finalizer` + // callbacks. The reason is that addons might store other references which + // they delete during their `napi_finalizer` callbacks. If we deleted such + // references here first, they would be doubly deleted when the + // `napi_finalizer` deleted them subsequently. + v8impl::RefTracker::FinalizeAll(&finalizing_reflist); + v8impl::RefTracker::FinalizeAll(&reflist); if (instance_data.finalize_cb != nullptr) { CallIntoModuleThrow([&](napi_env env) { instance_data.finalize_cb(env, instance_data.data, instance_data.hint); @@ -55,6 +62,12 @@ struct napi_env__ { } v8impl::Persistent last_exception; + + // We store references in two different lists, depending on whether they have + // `napi_finalizer` callbacks, because we must first finalize the ones that + // have such a callback. See `~napi_env__()` above for details. + v8impl::RefTracker::RefList reflist; + v8impl::RefTracker::RefList finalizing_reflist; napi_extended_error_info last_error; int open_handle_scopes = 0; int open_callback_scopes = 0; diff --git a/src/js_native_api_v8_internals.h b/src/js_native_api_v8_internals.h index ddd219818cdfa9..74afd1172e527e 100644 --- a/src/js_native_api_v8_internals.h +++ b/src/js_native_api_v8_internals.h @@ -28,6 +28,45 @@ namespace v8impl { +class RefTracker { + public: + RefTracker() {} + virtual ~RefTracker() {} + virtual void Finalize(bool isEnvTeardown) {} + + typedef RefTracker RefList; + + inline void Link(RefList* list) { + prev_ = list; + next_ = list->next_; + if (next_ != nullptr) { + next_->prev_ = this; + } + list->next_ = this; + } + + inline void Unlink() { + if (prev_ != nullptr) { + prev_->next_ = next_; + } + if (next_ != nullptr) { + next_->prev_ = prev_; + } + prev_ = nullptr; + next_ = nullptr; + } + + static void FinalizeAll(RefList* list) { + while (list->next_ != nullptr) { + list->next_->Finalize(true); + } + } + + private: + RefList* next_ = nullptr; + RefList* prev_ = nullptr; +}; + template using Persistent = v8::Global; diff --git a/test/js-native-api/test_general/testEnvCleanup.js b/test/js-native-api/test_general/testEnvCleanup.js new file mode 100644 index 00000000000000..8d567bef4518ce --- /dev/null +++ b/test/js-native-api/test_general/testEnvCleanup.js @@ -0,0 +1,57 @@ +'use strict'; + +if (process.argv[2] === 'child') { + const common = require('../../common'); + const test_general = require(`./build/${common.buildType}/test_general`); + + // The second argument to `envCleanupWrap()` is an index into the global + // static string array named `env_cleanup_finalizer_messages` on the native + // side. A reverse mapping is reproduced here for clarity. + const finalizerMessages = { + 'simple wrap': 0, + 'wrap, removeWrap': 1, + 'first wrap': 2, + 'second wrap': 3 + }; + + // We attach the three objects we will test to `module.exports` to ensure they + // will not be garbage-collected before the process exits. + + // Make sure the finalizer for a simple wrap will be called at env cleanup. + module.exports['simple wrap'] = + test_general.envCleanupWrap({}, finalizerMessages['simple wrap']); + + // Make sure that a removed wrap does not result in a call to its finalizer at + // env cleanup. + module.exports['wrap, removeWrap'] = + test_general.envCleanupWrap({}, finalizerMessages['wrap, removeWrap']); + test_general.removeWrap(module.exports['wrap, removeWrap']); + + // Make sure that only the latest attached version of a re-wrapped item's + // finalizer gets called at env cleanup. + module.exports['first wrap'] = + test_general.envCleanupWrap({}, finalizerMessages['first wrap']), + test_general.removeWrap(module.exports['first wrap']); + test_general.envCleanupWrap(module.exports['first wrap'], + finalizerMessages['second wrap']); +} else { + const assert = require('assert'); + const { spawnSync } = require('child_process'); + + const child = spawnSync(process.execPath, [__filename, 'child'], { + stdio: [ process.stdin, 'pipe', process.stderr ] + }); + + // Grab the child's output and construct an object whose keys are the rows of + // the output and whose values are `true`, so we can compare the output while + // ignoring the order in which the lines of it were produced. + assert.deepStrictEqual( + child.stdout.toString().split(/\r\n|\r|\n/g).reduce((obj, item) => + Object.assign(obj, item ? { [item]: true } : {}), {}), { + 'finalize at env cleanup for simple wrap': true, + 'finalize at env cleanup for second wrap': true + }); + + // Ensure that the child exited successfully. + assert.strictEqual(child.status, 0); +} diff --git a/test/js-native-api/test_general/test_general.c b/test/js-native-api/test_general/test_general.c index a7453e42f7456b..f6e641167d5bcc 100644 --- a/test/js-native-api/test_general/test_general.c +++ b/test/js-native-api/test_general/test_general.c @@ -1,5 +1,7 @@ -#include +#include #include +#include +#include #include "../common.h" static napi_value testStrictEquals(napi_env env, napi_callback_info info) { @@ -146,16 +148,22 @@ static napi_value deref_item_was_called(napi_env env, napi_callback_info info) { return it_was_called; } -static napi_value wrap(napi_env env, napi_callback_info info) { +static napi_value wrap_first_arg(napi_env env, + napi_callback_info info, + napi_finalize finalizer, + void* data) { size_t argc = 1; napi_value to_wrap; - deref_item_called = false; - NAPI_CALL(env, napi_get_cb_info(env, info, &argc, &to_wrap, NULL, NULL)); - NAPI_CALL(env, napi_wrap(env, to_wrap, &deref_item_called, deref_item, NULL, NULL)); + NAPI_CALL(env, napi_wrap(env, to_wrap, data, finalizer, NULL, NULL)); - return NULL; + return to_wrap; +} + +static napi_value wrap(napi_env env, napi_callback_info info) { + deref_item_called = false; + return wrap_first_arg(env, info, deref_item, &deref_item_called); } static napi_value unwrap(napi_env env, napi_callback_info info) { @@ -186,13 +194,7 @@ static void test_finalize(napi_env env, void* data, void* hint) { } static napi_value test_finalize_wrap(napi_env env, napi_callback_info info) { - size_t argc = 1; - napi_value js_object; - - NAPI_CALL(env, napi_get_cb_info(env, info, &argc, &js_object, NULL, NULL)); - NAPI_CALL(env, napi_wrap(env, js_object, NULL, test_finalize, NULL, NULL)); - - return NULL; + return wrap_first_arg(env, info, test_finalize, NULL); } static napi_value finalize_was_called(napi_env env, napi_callback_info info) { @@ -251,6 +253,34 @@ static napi_value add_finalizer_only(napi_env env, napi_callback_info info) { return NULL; } +static const char* env_cleanup_finalizer_messages[] = { + "simple wrap", + "wrap, removeWrap", + "first wrap", + "second wrap" +}; + +static void cleanup_env_finalizer(napi_env env, void* data, void* hint) { + (void) env; + (void) hint; + + printf("finalize at env cleanup for %s\n", + env_cleanup_finalizer_messages[(uintptr_t)data]); +} + +static napi_value env_cleanup_wrap(napi_env env, napi_callback_info info) { + size_t argc = 2; + napi_value argv[2]; + uint32_t value; + uintptr_t ptr_value; + NAPI_CALL(env, napi_get_cb_info(env, info, &argc, argv, NULL, NULL)); + + NAPI_CALL(env, napi_get_value_uint32(env, argv[1], &value)); + + ptr_value = value; + return wrap_first_arg(env, info, cleanup_env_finalizer, (void*)ptr_value); +} + EXTERN_C_START napi_value Init(napi_env env, napi_value exports) { napi_property_descriptor descriptors[] = { @@ -265,6 +295,7 @@ napi_value Init(napi_env env, napi_value exports) { DECLARE_NAPI_PROPERTY("testNapiErrorCleanup", testNapiErrorCleanup), DECLARE_NAPI_PROPERTY("testNapiTypeof", testNapiTypeof), DECLARE_NAPI_PROPERTY("wrap", wrap), + DECLARE_NAPI_PROPERTY("envCleanupWrap", env_cleanup_wrap), DECLARE_NAPI_PROPERTY("unwrap", unwrap), DECLARE_NAPI_PROPERTY("removeWrap", remove_wrap), DECLARE_NAPI_PROPERTY("addFinalizerOnly", add_finalizer_only),