diff options
-rw-r--r-- | node.gyp | 3 | ||||
-rw-r--r-- | src/async_wrap.cc | 4 | ||||
-rw-r--r-- | src/base_object-inl.h | 2 | ||||
-rw-r--r-- | src/base_object.h | 10 | ||||
-rw-r--r-- | src/env-inl.h | 4 | ||||
-rw-r--r-- | src/env.h | 5 | ||||
-rw-r--r-- | src/inspector_agent.cc | 1 | ||||
-rw-r--r-- | src/inspector_agent.h | 6 | ||||
-rw-r--r-- | src/inspector_js_api.cc | 1 | ||||
-rw-r--r-- | src/module_wrap.h | 8 | ||||
-rw-r--r-- | src/node_api.cc | 26 | ||||
-rw-r--r-- | src/node_buffer.cc | 1 | ||||
-rw-r--r-- | src/node_contextify.cc | 1 | ||||
-rw-r--r-- | src/node_contextify.h | 2 | ||||
-rw-r--r-- | src/node_crypto.cc | 1 | ||||
-rw-r--r-- | src/node_crypto.h | 4 | ||||
-rw-r--r-- | src/node_file.h | 1 | ||||
-rw-r--r-- | src/node_internals.h | 3 | ||||
-rw-r--r-- | src/node_persistent.h | 30 | ||||
-rw-r--r-- | src/node_zlib.cc | 1 | ||||
-rw-r--r-- | src/stream_base-inl.h | 26 | ||||
-rw-r--r-- | src/stream_base.h | 4 | ||||
-rw-r--r-- | src/stream_wrap.cc | 4 | ||||
-rw-r--r-- | src/util-inl.h | 8 | ||||
-rw-r--r-- | src/util.h | 7 |
25 files changed, 87 insertions, 76 deletions
@@ -371,9 +371,10 @@ 'src/node_internals.h', 'src/node_javascript.h', 'src/node_mutex.h', - 'src/node_platform.h', 'src/node_perf.h', 'src/node_perf_common.h', + 'src/node_persistent.h', + 'src/node_platform.h', 'src/node_root_certs.h', 'src/node_version.h', 'src/node_watchdog.h', diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 7fa5f0ade9..c23535f094 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -409,8 +409,8 @@ static void DisablePromiseHook(const FunctionCallbackInfo<Value>& args) { class DestroyParam { public: double asyncId; - v8::Persistent<Object> target; - v8::Persistent<Object> propBag; + Persistent<Object> target; + Persistent<Object> propBag; }; diff --git a/src/base_object-inl.h b/src/base_object-inl.h index 900fc2b3ed..6720bd6d88 100644 --- a/src/base_object-inl.h +++ b/src/base_object-inl.h @@ -47,7 +47,7 @@ inline BaseObject::~BaseObject() { } -inline v8::Persistent<v8::Object>& BaseObject::persistent() { +inline Persistent<v8::Object>& BaseObject::persistent() { return persistent_handle_; } diff --git a/src/base_object.h b/src/base_object.h index 965683d029..0640f91cbf 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -24,6 +24,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS +#include "node_persistent.h" #include "v8.h" namespace node { @@ -39,12 +40,7 @@ class BaseObject { // persistent.IsEmpty() is true. inline v8::Local<v8::Object> object(); - // The parent class is responsible for calling .Reset() on destruction - // when the persistent handle is strong because there is no way for - // BaseObject to know when the handle goes out of scope. - // Weak handles have been reset by the time the destructor runs but - // calling .Reset() again is harmless. - inline v8::Persistent<v8::Object>& persistent(); + inline Persistent<v8::Object>& persistent(); inline Environment* env() const; @@ -71,7 +67,7 @@ class BaseObject { // position of members in memory are predictable. For more information please // refer to `doc/guides/node-postmortem-support.md` friend int GenDebugSymbols(); - v8::Persistent<v8::Object> persistent_handle_; + Persistent<v8::Object> persistent_handle_; Environment* env_; }; diff --git a/src/env-inl.h b/src/env-inl.h index 2b2b0cba58..2aec83f503 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -550,8 +550,8 @@ void Environment::CreateImmediate(native_immediate_callback cb, native_immediate_callbacks_.push_back({ cb, data, - std::unique_ptr<v8::Persistent<v8::Object>>(obj.IsEmpty() ? - nullptr : new v8::Persistent<v8::Object>(isolate_, obj)), + std::unique_ptr<Persistent<v8::Object>>(obj.IsEmpty() ? + nullptr : new Persistent<v8::Object>(isolate_, obj)), ref }); immediate_info()->count_inc(1); @@ -847,7 +847,7 @@ class Environment { struct NativeImmediateCallback { native_immediate_callback cb_; void* data_; - std::unique_ptr<v8::Persistent<v8::Object>> keep_alive_; + std::unique_ptr<Persistent<v8::Object>> keep_alive_; bool refed_; }; std::vector<NativeImmediateCallback> native_immediate_callbacks_; @@ -858,8 +858,7 @@ class Environment { v8::Local<v8::Promise> promise, v8::Local<v8::Value> parent); -#define V(PropertyName, TypeName) \ - v8::Persistent<TypeName> PropertyName ## _; +#define V(PropertyName, TypeName) Persistent<TypeName> PropertyName ## _; ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V) #undef V diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index 7ba1a14471..e143d316d2 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -30,7 +30,6 @@ using v8::HandleScope; using v8::Isolate; using v8::Local; using v8::Object; -using v8::Persistent; using v8::String; using v8::Value; diff --git a/src/inspector_agent.h b/src/inspector_agent.h index 0555f5e18c..56fb407930 100644 --- a/src/inspector_agent.h +++ b/src/inspector_agent.h @@ -97,7 +97,7 @@ class Agent { private: void ToggleAsyncHook(v8::Isolate* isolate, - const v8::Persistent<v8::Function>& fn); + const Persistent<v8::Function>& fn); node::Environment* parent_env_; std::unique_ptr<NodeInspectorClient> client_; @@ -109,8 +109,8 @@ class Agent { bool pending_enable_async_hook_; bool pending_disable_async_hook_; - v8::Persistent<v8::Function> enable_async_hook_function_; - v8::Persistent<v8::Function> disable_async_hook_function_; + Persistent<v8::Function> enable_async_hook_function_; + Persistent<v8::Function> disable_async_hook_function_; }; } // namespace inspector diff --git a/src/inspector_js_api.cc b/src/inspector_js_api.cc index 428d8391f2..9a4857f1a1 100644 --- a/src/inspector_js_api.cc +++ b/src/inspector_js_api.cc @@ -19,7 +19,6 @@ using v8::Local; using v8::MaybeLocal; using v8::NewStringType; using v8::Object; -using v8::Persistent; using v8::String; using v8::Value; diff --git a/src/module_wrap.h b/src/module_wrap.h index ee3740b561..3969e2a378 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -62,11 +62,11 @@ class ModuleWrap : public BaseObject { static ModuleWrap* GetFromModule(node::Environment*, v8::Local<v8::Module>); - v8::Persistent<v8::Module> module_; - v8::Persistent<v8::String> url_; + Persistent<v8::Module> module_; + Persistent<v8::String> url_; bool linked_ = false; - std::unordered_map<std::string, v8::Persistent<v8::Promise>> resolve_cache_; - v8::Persistent<v8::Context> context_; + std::unordered_map<std::string, Persistent<v8::Promise>> resolve_cache_; + Persistent<v8::Context> context_; }; } // namespace loader diff --git a/src/node_api.cc b/src/node_api.cc index 2c5f3066f7..7b0e110579 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -38,10 +38,10 @@ struct napi_env__ { accessor_data_template.Reset(); } v8::Isolate* isolate; - v8::Persistent<v8::Value> last_exception; - v8::Persistent<v8::ObjectTemplate> wrap_template; - v8::Persistent<v8::ObjectTemplate> function_data_template; - v8::Persistent<v8::ObjectTemplate> accessor_data_template; + node::Persistent<v8::Value> last_exception; + node::Persistent<v8::ObjectTemplate> wrap_template; + node::Persistent<v8::ObjectTemplate> function_data_template; + node::Persistent<v8::ObjectTemplate> accessor_data_template; napi_extended_error_info last_error; int open_handle_scopes = 0; int open_callback_scopes = 0; @@ -274,13 +274,13 @@ static_assert(sizeof(v8::Local<v8::Value>) == sizeof(napi_value), "Cannot convert between v8::Local<v8::Value> and napi_value"); static -napi_deferred JsDeferredFromV8Persistent(v8::Persistent<v8::Value>* local) { +napi_deferred JsDeferredFromNodePersistent(node::Persistent<v8::Value>* local) { return reinterpret_cast<napi_deferred>(local); } static -v8::Persistent<v8::Value>* V8PersistentFromJsDeferred(napi_deferred local) { - return reinterpret_cast<v8::Persistent<v8::Value>*>(local); +node::Persistent<v8::Value>* NodePersistentFromJsDeferred(napi_deferred local) { + return reinterpret_cast<node::Persistent<v8::Value>*>(local); } static @@ -360,7 +360,7 @@ class Finalizer { void* _finalize_hint; }; -// Wrapper around v8::Persistent that implements reference counting. +// Wrapper around node::Persistent that implements reference counting. class Reference : private Finalizer { private: Reference(napi_env env, @@ -470,7 +470,7 @@ class Reference : private Finalizer { } } - v8::Persistent<v8::Value> _persistent; + node::Persistent<v8::Value> _persistent; uint32_t _refcount; bool _delete_self; }; @@ -846,8 +846,8 @@ napi_status ConcludeDeferred(napi_env env, CHECK_ARG(env, result); v8::Local<v8::Context> context = env->isolate->GetCurrentContext(); - v8::Persistent<v8::Value>* deferred_ref = - V8PersistentFromJsDeferred(deferred); + node::Persistent<v8::Value>* deferred_ref = + NodePersistentFromJsDeferred(deferred); v8::Local<v8::Value> v8_deferred = v8::Local<v8::Value>::New(env->isolate, *deferred_ref); @@ -3493,10 +3493,10 @@ napi_status napi_create_promise(napi_env env, CHECK_MAYBE_EMPTY(env, maybe, napi_generic_failure); auto v8_resolver = maybe.ToLocalChecked(); - auto v8_deferred = new v8::Persistent<v8::Value>(); + auto v8_deferred = new node::Persistent<v8::Value>(); v8_deferred->Reset(env->isolate, v8_resolver); - *deferred = v8impl::JsDeferredFromV8Persistent(v8_deferred); + *deferred = v8impl::JsDeferredFromNodePersistent(v8_deferred); *promise = v8impl::JsValueFromV8LocalValue(v8_resolver->GetPromise()); return GET_RETURN_STATUS(env); } diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 72776304c8..c511562b80 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -78,7 +78,6 @@ using v8::Local; using v8::Maybe; using v8::MaybeLocal; using v8::Object; -using v8::Persistent; using v8::String; using v8::Uint32Array; using v8::Uint8Array; diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 35b39d62e1..6966fac6f6 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -48,7 +48,6 @@ using v8::NamedPropertyHandlerConfiguration; using v8::Nothing; using v8::Object; using v8::ObjectTemplate; -using v8::Persistent; using v8::PropertyAttribute; using v8::PropertyCallbackInfo; using v8::PropertyDescriptor; diff --git a/src/node_contextify.h b/src/node_contextify.h index b2f32850ac..5c38c5d363 100644 --- a/src/node_contextify.h +++ b/src/node_contextify.h @@ -15,7 +15,7 @@ class ContextifyContext { enum { kSandboxObjectIndex = 1 }; Environment* const env_; - v8::Persistent<v8::Context> context_; + Persistent<v8::Context> context_; public: ContextifyContext(Environment* env, diff --git a/src/node_crypto.cc b/src/node_crypto.cc index a0b21e7c39..1ae744d6af 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -100,7 +100,6 @@ using v8::MaybeLocal; using v8::Null; using v8::Object; using v8::ObjectTemplate; -using v8::Persistent; using v8::PropertyAttribute; using v8::ReadOnly; using v8::Signature; diff --git a/src/node_crypto.h b/src/node_crypto.h index 7ce40697d4..1c17dc1081 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -353,11 +353,11 @@ class SSLWrap { ClientHelloParser hello_parser_; #ifdef NODE__HAVE_TLSEXT_STATUS_CB - v8::Persistent<v8::Object> ocsp_response_; + Persistent<v8::Object> ocsp_response_; #endif // NODE__HAVE_TLSEXT_STATUS_CB #ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB - v8::Persistent<v8::Value> sni_context_; + Persistent<v8::Value> sni_context_; #endif friend class SecureContext; diff --git a/src/node_file.h b/src/node_file.h index 094963c53a..a65b9f0e5f 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -14,7 +14,6 @@ using v8::HandleScope; using v8::Local; using v8::MaybeLocal; using v8::Object; -using v8::Persistent; using v8::Promise; using v8::Undefined; using v8::Value; diff --git a/src/node_internals.h b/src/node_internals.h index 094fcc2d83..140826bb7c 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -25,6 +25,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #include "node.h" +#include "node_persistent.h" #include "util-inl.h" #include "env-inl.h" #include "uv.h" @@ -215,7 +216,7 @@ class Environment; template <class TypeName> inline v8::Local<TypeName> PersistentToLocal( v8::Isolate* isolate, - const v8::Persistent<TypeName>& persistent); + const Persistent<TypeName>& persistent); // Creates a new context with Node.js-specific tweaks. Currently, it removes // the `v8BreakIterator` property from the global `Intl` object if present. diff --git a/src/node_persistent.h b/src/node_persistent.h new file mode 100644 index 0000000000..762842dd4b --- /dev/null +++ b/src/node_persistent.h @@ -0,0 +1,30 @@ +#ifndef SRC_NODE_PERSISTENT_H_ +#define SRC_NODE_PERSISTENT_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#include "v8.h" + +namespace node { + +template <typename T> +struct ResetInDestructorPersistentTraits { + static const bool kResetInDestructor = true; + template <typename S, typename M> + // Disallow copy semantics by leaving this unimplemented. + inline static void Copy( + const v8::Persistent<S, M>&, + v8::Persistent<T, ResetInDestructorPersistentTraits<T>>*); +}; + +// v8::Persistent does not reset the object slot in its destructor. That is +// acknowledged as a flaw in the V8 API and expected to change in the future +// but for now node::Persistent is the easier and safer alternative. +template <typename T> +using Persistent = v8::Persistent<T, ResetInDestructorPersistentTraits<T>>; + +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#endif // SRC_NODE_PERSISTENT_H_ diff --git a/src/node_zlib.cc b/src/node_zlib.cc index 8ef4383e03..388630d507 100644 --- a/src/node_zlib.cc +++ b/src/node_zlib.cc @@ -46,7 +46,6 @@ using v8::HandleScope; using v8::Local; using v8::Number; using v8::Object; -using v8::Persistent; using v8::String; using v8::Uint32Array; using v8::Value; diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h index b479e04bae..c864feb927 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -225,8 +225,8 @@ inline StreamWriteResult StreamBase::Write( return StreamWriteResult { async, err, req_wrap }; } -template<typename OtherBase, bool kResetPersistent> -SimpleShutdownWrap<OtherBase, kResetPersistent>::SimpleShutdownWrap( +template <typename OtherBase> +SimpleShutdownWrap<OtherBase>::SimpleShutdownWrap( StreamBase* stream, v8::Local<v8::Object> req_wrap_obj) : ShutdownWrap(stream, req_wrap_obj), @@ -236,14 +236,9 @@ SimpleShutdownWrap<OtherBase, kResetPersistent>::SimpleShutdownWrap( Wrap(req_wrap_obj, static_cast<AsyncWrap*>(this)); } -template<typename OtherBase, bool kResetPersistent> -SimpleShutdownWrap<OtherBase, kResetPersistent>::~SimpleShutdownWrap() { +template <typename OtherBase> +SimpleShutdownWrap<OtherBase>::~SimpleShutdownWrap() { ClearWrap(static_cast<AsyncWrap*>(this)->object()); - if (kResetPersistent) { - auto& persistent = static_cast<AsyncWrap*>(this)->persistent(); - CHECK_EQ(persistent.IsEmpty(), false); - persistent.Reset(); - } } inline ShutdownWrap* StreamBase::CreateShutdownWrap( @@ -251,8 +246,8 @@ inline ShutdownWrap* StreamBase::CreateShutdownWrap( return new SimpleShutdownWrap<AsyncWrap>(this, object); } -template<typename OtherBase, bool kResetPersistent> -SimpleWriteWrap<OtherBase, kResetPersistent>::SimpleWriteWrap( +template <typename OtherBase> +SimpleWriteWrap<OtherBase>::SimpleWriteWrap( StreamBase* stream, v8::Local<v8::Object> req_wrap_obj) : WriteWrap(stream, req_wrap_obj), @@ -262,14 +257,9 @@ SimpleWriteWrap<OtherBase, kResetPersistent>::SimpleWriteWrap( Wrap(req_wrap_obj, static_cast<AsyncWrap*>(this)); } -template<typename OtherBase, bool kResetPersistent> -SimpleWriteWrap<OtherBase, kResetPersistent>::~SimpleWriteWrap() { +template <typename OtherBase> +SimpleWriteWrap<OtherBase>::~SimpleWriteWrap() { ClearWrap(static_cast<AsyncWrap*>(this)->object()); - if (kResetPersistent) { - auto& persistent = static_cast<AsyncWrap*>(this)->persistent(); - CHECK_EQ(persistent.IsEmpty(), false); - persistent.Reset(); - } } inline WriteWrap* StreamBase::CreateWriteWrap( diff --git a/src/stream_base.h b/src/stream_base.h index 59b8ee7b72..8af05059f4 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -322,7 +322,7 @@ class StreamBase : public StreamResource { // `OtherBase` must have a constructor that matches the `AsyncWrap` // constructors’s (Environment*, Local<Object>, AsyncWrap::Provider) signature // and be a subclass of `AsyncWrap`. -template <typename OtherBase, bool kResetPersistentOnDestroy = true> +template <typename OtherBase> class SimpleShutdownWrap : public ShutdownWrap, public OtherBase { public: SimpleShutdownWrap(StreamBase* stream, @@ -333,7 +333,7 @@ class SimpleShutdownWrap : public ShutdownWrap, public OtherBase { size_t self_size() const override { return sizeof(*this); } }; -template <typename OtherBase, bool kResetPersistentOnDestroy = true> +template <typename OtherBase> class SimpleWriteWrap : public WriteWrap, public OtherBase { public: SimpleWriteWrap(StreamBase* stream, diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index e1df9edd39..27fe48d116 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -264,8 +264,8 @@ void LibuvStreamWrap::SetBlocking(const FunctionCallbackInfo<Value>& args) { args.GetReturnValue().Set(uv_stream_set_blocking(wrap->stream(), enable)); } -typedef SimpleShutdownWrap<ReqWrap<uv_shutdown_t>, false> LibuvShutdownWrap; -typedef SimpleWriteWrap<ReqWrap<uv_write_t>, false> LibuvWriteWrap; +typedef SimpleShutdownWrap<ReqWrap<uv_shutdown_t>> LibuvShutdownWrap; +typedef SimpleWriteWrap<ReqWrap<uv_write_t>> LibuvWriteWrap; ShutdownWrap* LibuvStreamWrap::CreateShutdownWrap(Local<Object> object) { return new LibuvShutdownWrap(this, object); diff --git a/src/util-inl.h b/src/util-inl.h index c5a25c91ff..d07cfea922 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -168,7 +168,7 @@ inline ContainerOfHelper<Inner, Outer> ContainerOf(Inner Outer::*field, template <class TypeName> inline v8::Local<TypeName> PersistentToLocal( v8::Isolate* isolate, - const v8::Persistent<TypeName>& persistent) { + const Persistent<TypeName>& persistent) { if (persistent.IsWeak()) { return WeakPersistentToLocal(isolate, persistent); } else { @@ -178,15 +178,15 @@ inline v8::Local<TypeName> PersistentToLocal( template <class TypeName> inline v8::Local<TypeName> StrongPersistentToLocal( - const v8::Persistent<TypeName>& persistent) { + const Persistent<TypeName>& persistent) { return *reinterpret_cast<v8::Local<TypeName>*>( - const_cast<v8::Persistent<TypeName>*>(&persistent)); + const_cast<Persistent<TypeName>*>(&persistent)); } template <class TypeName> inline v8::Local<TypeName> WeakPersistentToLocal( v8::Isolate* isolate, - const v8::Persistent<TypeName>& persistent) { + const Persistent<TypeName>& persistent) { return v8::Local<TypeName>::New(isolate, persistent); } diff --git a/src/util.h b/src/util.h index 21c566a4ca..7c679952d5 100644 --- a/src/util.h +++ b/src/util.h @@ -24,6 +24,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS +#include "node_persistent.h" #include "v8.h" #include <assert.h> @@ -218,7 +219,7 @@ inline ContainerOfHelper<Inner, Outer> ContainerOf(Inner Outer::*field, template <class TypeName> inline v8::Local<TypeName> PersistentToLocal( v8::Isolate* isolate, - const v8::Persistent<TypeName>& persistent); + const Persistent<TypeName>& persistent); // Unchecked conversion from a non-weak Persistent<T> to Local<TLocal<T>, // use with care! @@ -227,12 +228,12 @@ inline v8::Local<TypeName> PersistentToLocal( // scope, it will destroy the reference to the object. template <class TypeName> inline v8::Local<TypeName> StrongPersistentToLocal( - const v8::Persistent<TypeName>& persistent); + const Persistent<TypeName>& persistent); template <class TypeName> inline v8::Local<TypeName> WeakPersistentToLocal( v8::Isolate* isolate, - const v8::Persistent<TypeName>& persistent); + const Persistent<TypeName>& persistent); // Convenience wrapper around v8::String::NewFromOneByte(). inline v8::Local<v8::String> OneByteString(v8::Isolate* isolate, |