From 5f104e3218f7b71f6fa9ba3b98a272843685568c Mon Sep 17 00:00:00 2001 From: legendecas Date: Tue, 8 Jun 2021 21:53:07 +0800 Subject: [PATCH] node-api: cctest on v8impl::Reference PR-URL: https://github.com/nodejs/node/pull/38970 Backport-PR-URL: https://github.com/nodejs/node/pull/42512 Reviewed-By: Michael Dawson Reviewed-By: Gabriel Schulhof --- LICENSE | 2 +- node.gyp | 2 + src/gtest/LICENSE | 28 ++ src/gtest/gtest_prod.h | 61 +++ src/js_native_api_v8.cc | 556 +++++++++++++-------------- src/js_native_api_v8.h | 75 ++++ src/js_native_api_v8_internals.h | 1 + src/node_api.cc | 68 ++-- src/node_api_internals.h | 30 ++ test/cctest/test_js_native_api_v8.cc | 102 +++++ test/cctest/test_node_api.cc | 41 ++ 11 files changed, 635 insertions(+), 331 deletions(-) create mode 100644 src/gtest/LICENSE create mode 100644 src/gtest/gtest_prod.h create mode 100644 src/node_api_internals.h create mode 100644 test/cctest/test_js_native_api_v8.cc create mode 100644 test/cctest/test_node_api.cc diff --git a/LICENSE b/LICENSE index 298631ee09e2dd..e18293b3628173 100644 --- a/LICENSE +++ b/LICENSE @@ -1294,7 +1294,7 @@ The externally maintained libraries used by Node.js are: WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. """ -- gtest, located at test/cctest/gtest, is licensed as follows: +- gtest, located at src/gtest and test/cctest/gtest, is licensed as follows: """ Copyright 2008, Google Inc. All rights reserved. diff --git a/node.gyp b/node.gyp index 48fd7a124a6961..3bb6ce32fab8e5 100644 --- a/node.gyp +++ b/node.gyp @@ -1232,7 +1232,9 @@ 'test/cctest/test_base_object_ptr.cc', 'test/cctest/test_node_postmortem_metadata.cc', 'test/cctest/test_environment.cc', + 'test/cctest/test_js_native_api_v8.cc', 'test/cctest/test_linked_binding.cc', + 'test/cctest/test_node_api.cc', 'test/cctest/test_per_process.cc', 'test/cctest/test_platform.cc', 'test/cctest/test_json_utils.cc', diff --git a/src/gtest/LICENSE b/src/gtest/LICENSE new file mode 100644 index 00000000000000..1941a11f8ce943 --- /dev/null +++ b/src/gtest/LICENSE @@ -0,0 +1,28 @@ +Copyright 2008, Google Inc. +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + * Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above +copyright notice, this list of conditions and the following disclaimer +in the documentation and/or other materials provided with the +distribution. + * Neither the name of Google Inc. nor the names of its +contributors may be used to endorse or promote products derived from +this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/src/gtest/gtest_prod.h b/src/gtest/gtest_prod.h new file mode 100644 index 00000000000000..473387f170c838 --- /dev/null +++ b/src/gtest/gtest_prod.h @@ -0,0 +1,61 @@ +// Copyright 2006, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// +// Google C++ Testing and Mocking Framework definitions useful in production +// code. GOOGLETEST_CM0003 DO NOT DELETE + +#ifndef GOOGLETEST_INCLUDE_GTEST_GTEST_PROD_H_ // NOLINT +#define GOOGLETEST_INCLUDE_GTEST_GTEST_PROD_H_ + +// When you need to test the private or protected members of a class, +// use the FRIEND_TEST macro to declare your tests as friends of the +// class. For example: +// +// class MyClass { +// private: +// void PrivateMethod(); +// FRIEND_TEST(MyClassTest, PrivateMethodWorks); +// }; +// +// class MyClassTest : public testing::Test { +// // ... +// }; +// +// TEST_F(MyClassTest, PrivateMethodWorks) { +// // Can call MyClass::PrivateMethod() here. +// } +// +// Note: The test class must be in the same namespace as the class being tested. +// For example, putting MyClassTest in an anonymous namespace will not work. + +#define FRIEND_TEST(test_case_name, test_name) \ + friend class test_case_name##_##test_name##_Test + +#endif // GOOGLETEST_INCLUDE_GTEST_GTEST_PROD_H_ // NOLINT diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 8401ed30a64748..d8239fb1cb1c36 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -188,295 +188,6 @@ inline static napi_status ConcludeDeferred(napi_env env, return GET_RETURN_STATUS(env); } -// Wrapper around v8impl::Persistent that implements reference counting. -class RefBase : protected Finalizer, RefTracker { - protected: - RefBase(napi_env env, - uint32_t initial_refcount, - bool delete_self, - napi_finalize finalize_callback, - void* finalize_data, - void* finalize_hint) - : Finalizer(env, finalize_callback, finalize_data, finalize_hint), - _refcount(initial_refcount), - _delete_self(delete_self) { - Link(finalize_callback == nullptr - ? &env->reflist - : &env->finalizing_reflist); - } - - public: - static RefBase* New(napi_env env, - uint32_t initial_refcount, - bool delete_self, - napi_finalize finalize_callback, - void* finalize_data, - void* finalize_hint) { - return new RefBase(env, - initial_refcount, - delete_self, - finalize_callback, - finalize_data, - finalize_hint); - } - - inline void* Data() { - return _finalize_data; - } - - // Delete is called in 2 ways. Either from the finalizer or - // from one of Unwrap or napi_delete_reference. - // - // When it is called from Unwrap or napi_delete_reference we only - // want to do the delete if the finalizer has already run or - // cannot have been queued to run (ie the reference count is > 0), - // otherwise we may crash when the finalizer does run. - // If the finalizer may have been queued and has not already run - // delay the delete until the finalizer runs by not doing the delete - // and setting _delete_self to true so that the finalizer will - // delete it when it runs. - // - // The second way this is called is from - // the finalizer and _delete_self is set. In this case we - // know we need to do the deletion so just do it. - static inline void Delete(RefBase* reference) { - reference->Unlink(); - if ((reference->RefCount() != 0) || - (reference->_delete_self) || - (reference->_finalize_ran)) { - delete reference; - } else { - // defer until finalizer runs as - // it may alread be queued - reference->_delete_self = true; - } - } - - inline uint32_t Ref() { - return ++_refcount; - } - - inline uint32_t Unref() { - if (_refcount == 0) { - return 0; - } - return --_refcount; - } - - inline uint32_t RefCount() { - return _refcount; - } - - protected: - inline void Finalize(bool is_env_teardown = false) override { - // In addition to being called during environment teardown, this method is - // also the entry point for the garbage collector. During environment - // teardown we have to remove the garbage collector's reference to this - // method so that, if, as part of the user's callback, JS gets executed, - // resulting in a garbage collection pass, this method is not re-entered as - // part of that pass, because that'll cause a double free (as seen in - // https://github.com/nodejs/node/issues/37236). - // - // Since this class does not have access to the V8 persistent reference, - // this method is overridden in the `Reference` class below. Therein the - // weak callback is removed, ensuring that the garbage collector does not - // re-enter this method, and the method chains up to continue the process of - // environment-teardown-induced finalization. - - // During environment teardown we have to convert a strong reference to - // a weak reference to force the deferring behavior if the user's finalizer - // happens to delete this reference so that the code in this function that - // follows the call to the user's finalizer may safely access variables from - // this instance. - if (is_env_teardown && RefCount() > 0) _refcount = 0; - - if (_finalize_callback != nullptr) { - // This ensures that we never call the finalizer twice. - napi_finalize fini = _finalize_callback; - _finalize_callback = nullptr; - _env->CallFinalizer(fini, _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; - } - } - - private: - uint32_t _refcount; - bool _delete_self; -}; - -class Reference : public RefBase { - using SecondPassCallParameterRef = Reference*; - - protected: - template - Reference(napi_env env, v8::Local value, Args&&... args) - : RefBase(env, std::forward(args)...), - _persistent(env->isolate, value), - _secondPassParameter(new SecondPassCallParameterRef(this)), - _secondPassScheduled(false) { - if (RefCount() == 0) { - SetWeak(); - } - } - - public: - static inline Reference* New(napi_env env, - v8::Local value, - uint32_t initial_refcount, - bool delete_self, - napi_finalize finalize_callback = nullptr, - void* finalize_data = nullptr, - void* finalize_hint = nullptr) { - return new Reference(env, - value, - initial_refcount, - delete_self, - finalize_callback, - finalize_data, - finalize_hint); - } - - virtual ~Reference() { - // If the second pass callback is scheduled, it will delete the - // parameter passed to it, otherwise it will never be scheduled - // and we need to delete it here. - if (!_secondPassScheduled) { - delete _secondPassParameter; - } - } - - inline uint32_t Ref() { - uint32_t refcount = RefBase::Ref(); - if (refcount == 1) { - ClearWeak(); - } - return refcount; - } - - inline uint32_t Unref() { - uint32_t old_refcount = RefCount(); - uint32_t refcount = RefBase::Unref(); - if (old_refcount == 1 && refcount == 0) { - SetWeak(); - } - return refcount; - } - - inline v8::Local Get() { - if (_persistent.IsEmpty()) { - return v8::Local(); - } else { - return v8::Local::New(_env->isolate, _persistent); - } - } - - protected: - inline void Finalize(bool is_env_teardown = false) override { - if (is_env_teardown) env_teardown_finalize_started_ = true; - if (!is_env_teardown && env_teardown_finalize_started_) return; - - // During env teardown, `~napi_env()` alone is responsible for finalizing. - // Thus, we don't want any stray gc passes to trigger a second call to - // `RefBase::Finalize()`. ClearWeak will ensure that even if the - // gc is in progress no Finalization will be run for this Reference - // by the gc. - if (is_env_teardown) { - ClearWeak(); - } - - // Chain up to perform the rest of the finalization. - RefBase::Finalize(is_env_teardown); - } - - private: - // ClearWeak is marking the Reference so that the gc should not - // collect it, but it is possible that a second pass callback - // may have been scheduled already if we are in shutdown. We clear - // the secondPassParameter so that even if it has been - // secheduled no Finalization will be run. - inline void ClearWeak() { - if (!_persistent.IsEmpty()) { - _persistent.ClearWeak(); - } - if (_secondPassParameter != nullptr) { - *_secondPassParameter = nullptr; - } - } - - // Mark the reference as weak and eligible for collection - // by the gc. - inline void SetWeak() { - if (_secondPassParameter == nullptr) { - // This means that the Reference has already been processed - // by the second pass calback, so its already been Finalized, do - // nothing - return; - } - _persistent.SetWeak( - _secondPassParameter, FinalizeCallback, - v8::WeakCallbackType::kParameter); - *_secondPassParameter = this; - } - - // 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 - // a finalizer which may make calls back into the engine by allowing us to - // attach such a second-pass finalizer from the first pass finalizer. Thus, - // we do that here to ensure that the N-API finalizer callback is free to call - // into the engine. - static void FinalizeCallback( - const v8::WeakCallbackInfo& data) { - SecondPassCallParameterRef* parameter = data.GetParameter(); - Reference* reference = *parameter; - if (reference == nullptr) { - return; - } - - // The reference must be reset during the first pass. - reference->_persistent.Reset(); - // Mark the parameter not delete-able until the second pass callback is - // invoked. - reference->_secondPassScheduled = true; - data.SetSecondPassCallback(SecondPassCallback); - } - - // Second pass callbacks are scheduled with platform tasks. At env teardown, - // the tasks may have already be scheduled and we are unable to cancel the - // second pass callback task. We have to make sure that parameter is kept - // alive until the second pass callback is been invoked. In order to do - // this and still allow our code to Finalize/delete the Reference during - // shutdown we have to use a seperately allocated parameter instead - // of a parameter within the Reference object itself. This is what - // is stored in _secondPassParameter and it is alocated in the - // constructor for the Reference. - static void SecondPassCallback( - const v8::WeakCallbackInfo& data) { - SecondPassCallParameterRef* parameter = data.GetParameter(); - Reference* reference = *parameter; - delete parameter; - if (reference == nullptr) { - // the reference itself has already been deleted so nothing to do - return; - } - reference->_secondPassParameter = nullptr; - reference->Finalize(); - } - - bool env_teardown_finalize_started_ = false; - v8impl::Persistent _persistent; - SecondPassCallParameterRef* _secondPassParameter; - bool _secondPassScheduled; -}; - class ArrayBufferReference final : public Reference { public: // Same signatures for ctor and New() as Reference, except this only works @@ -823,6 +534,273 @@ inline napi_status Wrap(napi_env env, } // end of anonymous namespace +// Wrapper around v8impl::Persistent that implements reference counting. +RefBase::RefBase(napi_env env, + uint32_t initial_refcount, + bool delete_self, + napi_finalize finalize_callback, + void* finalize_data, + void* finalize_hint) + : Finalizer(env, finalize_callback, finalize_data, finalize_hint), + _refcount(initial_refcount), + _delete_self(delete_self) { + Link(finalize_callback == nullptr ? &env->reflist : &env->finalizing_reflist); +} + +RefBase* RefBase::New(napi_env env, + uint32_t initial_refcount, + bool delete_self, + napi_finalize finalize_callback, + void* finalize_data, + void* finalize_hint) { + return new RefBase(env, + initial_refcount, + delete_self, + finalize_callback, + finalize_data, + finalize_hint); +} + +RefBase::~RefBase() { + Unlink(); +} + +void* RefBase::Data() { + return _finalize_data; +} + +// Delete is called in 2 ways. Either from the finalizer or +// from one of Unwrap or napi_delete_reference. +// +// When it is called from Unwrap or napi_delete_reference we only +// want to do the delete if the finalizer has already run or +// cannot have been queued to run (ie the reference count is > 0), +// otherwise we may crash when the finalizer does run. +// If the finalizer may have been queued and has not already run +// delay the delete until the finalizer runs by not doing the delete +// and setting _delete_self to true so that the finalizer will +// delete it when it runs. +// +// The second way this is called is from +// the finalizer and _delete_self is set. In this case we +// know we need to do the deletion so just do it. +void RefBase::Delete(RefBase* reference) { + if ((reference->RefCount() != 0) || (reference->_delete_self) || + (reference->_finalize_ran)) { + delete reference; + } else { + // defer until finalizer runs as + // it may already be queued + reference->_delete_self = true; + } +} + +uint32_t RefBase::Ref() { + return ++_refcount; +} + +uint32_t RefBase::Unref() { + if (_refcount == 0) { + return 0; + } + return --_refcount; +} + +uint32_t RefBase::RefCount() { + return _refcount; +} + +void RefBase::Finalize(bool is_env_teardown) { + // In addition to being called during environment teardown, this method is + // also the entry point for the garbage collector. During environment + // teardown we have to remove the garbage collector's reference to this + // method so that, if, as part of the user's callback, JS gets executed, + // resulting in a garbage collection pass, this method is not re-entered as + // part of that pass, because that'll cause a double free (as seen in + // https://github.com/nodejs/node/issues/37236). + // + // Since this class does not have access to the V8 persistent reference, + // this method is overridden in the `Reference` class below. Therein the + // weak callback is removed, ensuring that the garbage collector does not + // re-enter this method, and the method chains up to continue the process of + // environment-teardown-induced finalization. + + // During environment teardown we have to convert a strong reference to + // a weak reference to force the deferring behavior if the user's finalizer + // happens to delete this reference so that the code in this function that + // follows the call to the user's finalizer may safely access variables from + // this instance. + if (is_env_teardown && RefCount() > 0) _refcount = 0; + + if (_finalize_callback != nullptr) { + // This ensures that we never call the finalizer twice. + napi_finalize fini = _finalize_callback; + _finalize_callback = nullptr; + _env->CallFinalizer(fini, _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; + } +} + +template +Reference::Reference(napi_env env, v8::Local value, Args&&... args) + : RefBase(env, std::forward(args)...), + _persistent(env->isolate, value), + _secondPassParameter(new SecondPassCallParameterRef(this)), + _secondPassScheduled(false) { + if (RefCount() == 0) { + SetWeak(); + } +} + +Reference* Reference::New(napi_env env, + v8::Local value, + uint32_t initial_refcount, + bool delete_self, + napi_finalize finalize_callback, + void* finalize_data, + void* finalize_hint) { + return new Reference(env, + value, + initial_refcount, + delete_self, + finalize_callback, + finalize_data, + finalize_hint); +} + +Reference::~Reference() { + // If the second pass callback is scheduled, it will delete the + // parameter passed to it, otherwise it will never be scheduled + // and we need to delete it here. + if (!_secondPassScheduled) { + delete _secondPassParameter; + } +} + +uint32_t Reference::Ref() { + uint32_t refcount = RefBase::Ref(); + if (refcount == 1) { + ClearWeak(); + } + return refcount; +} + +uint32_t Reference::Unref() { + uint32_t old_refcount = RefCount(); + uint32_t refcount = RefBase::Unref(); + if (old_refcount == 1 && refcount == 0) { + SetWeak(); + } + return refcount; +} + +v8::Local Reference::Get() { + if (_persistent.IsEmpty()) { + return v8::Local(); + } else { + return v8::Local::New(_env->isolate, _persistent); + } +} + +void Reference::Finalize(bool is_env_teardown) { + if (is_env_teardown) env_teardown_finalize_started_ = true; + if (!is_env_teardown && env_teardown_finalize_started_) return; + + // During env teardown, `~napi_env()` alone is responsible for finalizing. + // Thus, we don't want any stray gc passes to trigger a second call to + // `RefBase::Finalize()`. ClearWeak will ensure that even if the + // gc is in progress no Finalization will be run for this Reference + // by the gc. + if (is_env_teardown) { + ClearWeak(); + } + + // Chain up to perform the rest of the finalization. + RefBase::Finalize(is_env_teardown); +} + +// ClearWeak is marking the Reference so that the gc should not +// collect it, but it is possible that a second pass callback +// may have been scheduled already if we are in shutdown. We clear +// the secondPassParameter so that even if it has been +// scheduled no Finalization will be run. +void Reference::ClearWeak() { + if (!_persistent.IsEmpty()) { + _persistent.ClearWeak(); + } + if (_secondPassParameter != nullptr) { + *_secondPassParameter = nullptr; + } +} + +// Mark the reference as weak and eligible for collection +// by the gc. +void Reference::SetWeak() { + if (_secondPassParameter == nullptr) { + // This means that the Reference has already been processed + // by the second pass callback, so its already been Finalized, do + // nothing + return; + } + _persistent.SetWeak( + _secondPassParameter, FinalizeCallback, v8::WeakCallbackType::kParameter); + *_secondPassParameter = this; +} + +// 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 +// a finalizer which may make calls back into the engine by allowing us to +// attach such a second-pass finalizer from the first pass finalizer. Thus, +// we do that here to ensure that the N-API finalizer callback is free to call +// into the engine. +void Reference::FinalizeCallback( + const v8::WeakCallbackInfo& data) { + SecondPassCallParameterRef* parameter = data.GetParameter(); + Reference* reference = *parameter; + if (reference == nullptr) { + return; + } + + // The reference must be reset during the first pass. + reference->_persistent.Reset(); + // Mark the parameter not delete-able until the second pass callback is + // invoked. + reference->_secondPassScheduled = true; + + data.SetSecondPassCallback(SecondPassCallback); +} + +// Second pass callbacks are scheduled with platform tasks. At env teardown, +// the tasks may have already be scheduled and we are unable to cancel the +// second pass callback task. We have to make sure that parameter is kept +// alive until the second pass callback is been invoked. In order to do +// this and still allow our code to Finalize/delete the Reference during +// shutdown we have to use a separately allocated parameter instead +// of a parameter within the Reference object itself. This is what +// is stored in _secondPassParameter and it is allocated in the +// constructor for the Reference. +void Reference::SecondPassCallback( + const v8::WeakCallbackInfo& data) { + SecondPassCallParameterRef* parameter = data.GetParameter(); + Reference* reference = *parameter; + delete parameter; + if (reference == nullptr) { + // the reference itself has already been deleted so nothing to do + return; + } + reference->_secondPassParameter = nullptr; + reference->Finalize(); +} + } // end of namespace v8impl // Warning: Keep in-sync with napi_status enum diff --git a/src/js_native_api_v8.h b/src/js_native_api_v8.h index 95f390b58287e8..1ffbce8be2cefa 100644 --- a/src/js_native_api_v8.h +++ b/src/js_native_api_v8.h @@ -366,6 +366,81 @@ class TryCatch : public v8::TryCatch { napi_env _env; }; +// Wrapper around v8impl::Persistent that implements reference counting. +class RefBase : protected Finalizer, RefTracker { + protected: + RefBase(napi_env env, + uint32_t initial_refcount, + bool delete_self, + napi_finalize finalize_callback, + void* finalize_data, + void* finalize_hint); + + public: + static RefBase* New(napi_env env, + uint32_t initial_refcount, + bool delete_self, + napi_finalize finalize_callback, + void* finalize_data, + void* finalize_hint); + + static inline void Delete(RefBase* reference); + + virtual ~RefBase(); + void* Data(); + uint32_t Ref(); + uint32_t Unref(); + uint32_t RefCount(); + + protected: + void Finalize(bool is_env_teardown = false) override; + + private: + uint32_t _refcount; + bool _delete_self; +}; + +class Reference : public RefBase { + using SecondPassCallParameterRef = Reference*; + + protected: + template + Reference(napi_env env, v8::Local value, Args&&... args); + + public: + static Reference* New(napi_env env, + v8::Local value, + uint32_t initial_refcount, + bool delete_self, + napi_finalize finalize_callback = nullptr, + void* finalize_data = nullptr, + void* finalize_hint = nullptr); + + virtual ~Reference(); + uint32_t Ref(); + uint32_t Unref(); + v8::Local Get(); + + protected: + void Finalize(bool is_env_teardown = false) override; + + private: + void ClearWeak(); + void SetWeak(); + + static void FinalizeCallback( + const v8::WeakCallbackInfo& data); + static void SecondPassCallback( + const v8::WeakCallbackInfo& data); + + bool env_teardown_finalize_started_ = false; + v8impl::Persistent _persistent; + SecondPassCallParameterRef* _secondPassParameter; + bool _secondPassScheduled; + + FRIEND_TEST(JsNativeApiV8Test, Reference); +}; + } // end of namespace v8impl #endif // SRC_JS_NATIVE_API_V8_H_ diff --git a/src/js_native_api_v8_internals.h b/src/js_native_api_v8_internals.h index ddd219818cdfa9..8428390ef1eaf3 100644 --- a/src/js_native_api_v8_internals.h +++ b/src/js_native_api_v8_internals.h @@ -16,6 +16,7 @@ #include "node_version.h" #include "env.h" #include "node_internals.h" +#include "gtest/gtest_prod.h" #define NAPI_ARRAYSIZE(array) \ node::arraysize((array)) diff --git a/src/node_api.cc b/src/node_api.cc index a153e5dfcd204d..d5e05bcdd96ed0 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -2,6 +2,7 @@ #define NAPI_EXPERIMENTAL #include "js_native_api_v8.h" #include "node_api.h" +#include "node_api_internals.h" #include "node_binding.h" #include "node_buffer.h" #include "node_errors.h" @@ -11,51 +12,36 @@ #include -struct node_napi_env__ : public napi_env__ { - explicit node_napi_env__(v8::Local context, - const std::string& module_filename): - napi_env__(context), filename(module_filename) { - CHECK_NOT_NULL(node_env()); - } - - inline node::Environment* node_env() const { - return node::Environment::GetCurrent(context()); - } +node_napi_env__::node_napi_env__(v8::Local context, + const std::string& module_filename) + : napi_env__(context), filename(module_filename) { + CHECK_NOT_NULL(node_env()); +} - bool can_call_into_js() const override { - return node_env()->can_call_into_js(); - } +bool node_napi_env__::can_call_into_js() const { + return node_env()->can_call_into_js(); +} - v8::Maybe mark_arraybuffer_as_untransferable( - v8::Local ab) const override { - return ab->SetPrivate( - context(), - node_env()->untransferable_object_private_symbol(), - v8::True(isolate)); - } +v8::Maybe node_napi_env__::mark_arraybuffer_as_untransferable( + v8::Local ab) const { + return ab->SetPrivate(context(), + node_env()->untransferable_object_private_symbol(), + v8::True(isolate)); +} - void CallFinalizer(napi_finalize cb, void* data, void* hint) override { - // we need to keep the env live until the finalizer has been run - // EnvRefHolder provides an exception safe wrapper to Ref and then - // Unref once the lamba is freed - EnvRefHolder liveEnv(static_cast(this)); - node_env()->SetImmediate([=, liveEnv = std::move(liveEnv)] - (node::Environment* node_env) { - napi_env env = liveEnv.env(); - v8::HandleScope handle_scope(env->isolate); - v8::Context::Scope context_scope(env->context()); - env->CallIntoModule([&](napi_env env) { - cb(env, data, hint); +void node_napi_env__::CallFinalizer(napi_finalize cb, void* data, void* hint) { + // we need to keep the env live until the finalizer has been run + // EnvRefHolder provides an exception safe wrapper to Ref and then + // Unref once the lamba is freed + EnvRefHolder liveEnv(static_cast(this)); + node_env()->SetImmediate( + [=, liveEnv = std::move(liveEnv)](node::Environment* node_env) { + napi_env env = liveEnv.env(); + v8::HandleScope handle_scope(env->isolate); + v8::Context::Scope context_scope(env->context()); + env->CallIntoModule([&](napi_env env) { cb(env, data, hint); }); }); - }); - } - - const char* GetFilename() const { return filename.c_str(); } - - std::string filename; -}; - -typedef node_napi_env__* node_napi_env; +} namespace v8impl { diff --git a/src/node_api_internals.h b/src/node_api_internals.h new file mode 100644 index 00000000000000..318ada38083435 --- /dev/null +++ b/src/node_api_internals.h @@ -0,0 +1,30 @@ +#ifndef SRC_NODE_API_INTERNALS_H_ +#define SRC_NODE_API_INTERNALS_H_ + +#include "v8.h" +#define NAPI_EXPERIMENTAL +#include "env-inl.h" +#include "js_native_api_v8.h" +#include "node_api.h" +#include "util-inl.h" + +struct node_napi_env__ : public napi_env__ { + node_napi_env__(v8::Local context, + const std::string& module_filename); + + bool can_call_into_js() const override; + v8::Maybe mark_arraybuffer_as_untransferable( + v8::Local ab) const override; + void CallFinalizer(napi_finalize cb, void* data, void* hint) override; + + inline node::Environment* node_env() const { + return node::Environment::GetCurrent(context()); + } + inline const char* GetFilename() const { return filename.c_str(); } + + std::string filename; +}; + +using node_napi_env = node_napi_env__*; + +#endif // SRC_NODE_API_INTERNALS_H_ diff --git a/test/cctest/test_js_native_api_v8.cc b/test/cctest/test_js_native_api_v8.cc new file mode 100644 index 00000000000000..2d95c86d09d5d5 --- /dev/null +++ b/test/cctest/test_js_native_api_v8.cc @@ -0,0 +1,102 @@ +#include +#include +#include +#include "env-inl.h" +#include "gtest/gtest.h" +#include "js_native_api_v8.h" +#include "node_api_internals.h" +#include "node_binding.h" +#include "node_test_fixture.h" + +namespace v8impl { + +using v8::Local; +using v8::Object; + +static napi_env addon_env; +static uint32_t finalizer_call_count = 0; + +class JsNativeApiV8Test : public EnvironmentTestFixture { + private: + void SetUp() override { + EnvironmentTestFixture::SetUp(); + finalizer_call_count = 0; + } + + void TearDown() override { NodeTestFixture::TearDown(); } +}; + +TEST_F(JsNativeApiV8Test, Reference) { + const v8::HandleScope handle_scope(isolate_); + Argv argv; + + napi_ref ref; + void* embedder_fields[v8::kEmbedderFieldsInWeakCallback] = {nullptr, nullptr}; + v8::WeakCallbackInfo::Callback + callback; + Reference::SecondPassCallParameterRef* parameter = nullptr; + + { + Env test_env{handle_scope, argv}; + + node::Environment* env = *test_env; + node::LoadEnvironment(env, ""); + + napi_addon_register_func init = [](napi_env env, napi_value exports) { + addon_env = env; + return exports; + }; + Local module_obj = Object::New(isolate_); + Local exports_obj = Object::New(isolate_); + napi_module_register_by_symbol( + exports_obj, module_obj, env->context(), init); + ASSERT_NE(addon_env, nullptr); + node_napi_env internal_env = reinterpret_cast(addon_env); + EXPECT_EQ(internal_env->node_env(), env); + + // Create a new scope to manage the handles. + { + const v8::HandleScope handle_scope(isolate_); + napi_value value; + napi_create_object(addon_env, &value); + // Create a weak reference; + napi_add_finalizer( + addon_env, + value, + nullptr, + [](napi_env env, void* finalize_data, void* finalize_hint) { + finalizer_call_count++; + }, + nullptr, + &ref); + parameter = reinterpret_cast(ref)->_secondPassParameter; + } + + // We can hardly trigger a non-forced Garbage Collection in a stable way. + // Here we just invoke the weak callbacks directly. + // The persistant handles should be reset in the weak callback in respect + // to the API contract of v8 weak callbacks. + v8::WeakCallbackInfo data( + reinterpret_cast(isolate_), + parameter, + embedder_fields, + &callback); + Reference::FinalizeCallback(data); + EXPECT_EQ(callback, &Reference::SecondPassCallback); + } + // Env goes out of scope, the environment has been teardown. All node-api ref + // trackers should have been destroyed. + + // Now we call the second pass callback to verify the method do not abort with + // memory violations. + v8::WeakCallbackInfo data( + reinterpret_cast(isolate_), + parameter, + embedder_fields, + nullptr); + Reference::SecondPassCallback(data); + + // After Environment Teardown + EXPECT_EQ(finalizer_call_count, uint32_t(1)); +} +} // namespace v8impl diff --git a/test/cctest/test_node_api.cc b/test/cctest/test_node_api.cc new file mode 100644 index 00000000000000..ad5be52fc8ffcc --- /dev/null +++ b/test/cctest/test_node_api.cc @@ -0,0 +1,41 @@ +#include +#include +#include +#include "env-inl.h" +#include "gtest/gtest.h" +#include "node_api_internals.h" +#include "node_binding.h" +#include "node_test_fixture.h" + +using v8::Local; +using v8::Object; + +static napi_env addon_env; + +class NodeApiTest : public EnvironmentTestFixture { + private: + void SetUp() override { EnvironmentTestFixture::SetUp(); } + + void TearDown() override { NodeTestFixture::TearDown(); } +}; + +TEST_F(NodeApiTest, CreateNodeApiEnv) { + const v8::HandleScope handle_scope(isolate_); + Argv argv; + + Env test_env{handle_scope, argv}; + + node::Environment* env = *test_env; + node::LoadEnvironment(env, ""); + + napi_addon_register_func init = [](napi_env env, napi_value exports) { + addon_env = env; + return exports; + }; + Local module_obj = Object::New(isolate_); + Local exports_obj = Object::New(isolate_); + napi_module_register_by_symbol(exports_obj, module_obj, env->context(), init); + ASSERT_NE(addon_env, nullptr); + node_napi_env internal_env = reinterpret_cast(addon_env); + EXPECT_EQ(internal_env->node_env(), env); +}