diff options
author | Anna Henningsen <anna@addaleax.net> | 2019-04-17 23:16:22 +0200 |
---|---|---|
committer | Anna Henningsen <anna@addaleax.net> | 2019-04-30 00:23:33 +0200 |
commit | 723d5c058fa180684df13bd2a83bbf3ca6201957 (patch) | |
tree | 6aeb4bc88e6c437853327cced05ab797827c22f2 | |
parent | 095bd569aef4fec433ddb26e681eaabff1f7fd10 (diff) | |
download | android-node-v8-723d5c058fa180684df13bd2a83bbf3ca6201957.tar.gz android-node-v8-723d5c058fa180684df13bd2a83bbf3ca6201957.tar.bz2 android-node-v8-723d5c058fa180684df13bd2a83bbf3ca6201957.zip |
src: prefer v8::Global over node::Persistent
`v8::Global` is essentially a nicer variant of `node::Persistent` that,
in addition to reset-on-destroy, also implements move semantics.
This commit makes the necessary replacements, removes
`node::Persistent` and (now-)unnecessary inclusions of the
`node_persistent.h` header, and makes some of the functions that
take Persistents as arguments more generic so that they work with all
`v8::PersistentBase` flavours.
PR-URL: https://github.com/nodejs/node/pull/27287
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
-rw-r--r-- | node.gyp | 1 | ||||
-rw-r--r-- | src/async_wrap.cc | 5 | ||||
-rw-r--r-- | src/base_object-inl.h | 2 | ||||
-rw-r--r-- | src/base_object.h | 7 | ||||
-rw-r--r-- | src/env.h | 4 | ||||
-rw-r--r-- | src/heap_utils.cc | 3 | ||||
-rw-r--r-- | src/inspector_agent.cc | 3 | ||||
-rw-r--r-- | src/inspector_agent.h | 7 | ||||
-rw-r--r-- | src/inspector_js_api.cc | 3 | ||||
-rw-r--r-- | src/js_native_api_v8_internals.h | 2 | ||||
-rw-r--r-- | src/memory_tracker-inl.h | 4 | ||||
-rw-r--r-- | src/memory_tracker.h | 8 | ||||
-rw-r--r-- | src/module_wrap.cc | 3 | ||||
-rw-r--r-- | src/module_wrap.h | 8 | ||||
-rw-r--r-- | src/node_buffer.cc | 3 | ||||
-rw-r--r-- | src/node_contextify.h | 4 | ||||
-rw-r--r-- | src/node_crypto.h | 4 | ||||
-rw-r--r-- | src/node_file.h | 4 | ||||
-rw-r--r-- | src/node_internals.h | 1 | ||||
-rw-r--r-- | src/node_persistent.h | 66 | ||||
-rw-r--r-- | src/node_util.cc | 3 | ||||
-rw-r--r-- | src/node_zlib.cc | 3 | ||||
-rw-r--r-- | src/util.h | 39 |
23 files changed, 81 insertions, 106 deletions
@@ -606,7 +606,6 @@ 'src/node_options-inl.h', 'src/node_perf.h', 'src/node_perf_common.h', - 'src/node_persistent.h', 'src/node_platform.h', 'src/node_process.h', 'src/node_revert.h', diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 9cfa0924c7..e822255265 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -35,6 +35,7 @@ using v8::EscapableHandleScope; using v8::Function; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; +using v8::Global; using v8::HandleScope; using v8::Integer; using v8::Isolate; @@ -346,8 +347,8 @@ class DestroyParam { public: double asyncId; Environment* env; - Persistent<Object> target; - Persistent<Object> propBag; + Global<Object> target; + Global<Object> propBag; }; void AsyncWrap::WeakCallback(const WeakCallbackInfo<DestroyParam>& info) { diff --git a/src/base_object-inl.h b/src/base_object-inl.h index e3d645056a..fd61c15fea 100644 --- a/src/base_object-inl.h +++ b/src/base_object-inl.h @@ -56,7 +56,7 @@ BaseObject::~BaseObject() { } -Persistent<v8::Object>& BaseObject::persistent() { +v8::Global<v8::Object>& BaseObject::persistent() { return persistent_handle_; } diff --git a/src/base_object.h b/src/base_object.h index f1c666224f..cb83462ff5 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -24,7 +24,6 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include "node_persistent.h" #include "memory_tracker-inl.h" #include "v8.h" #include <type_traits> // std::remove_reference @@ -50,7 +49,7 @@ class BaseObject : public MemoryRetainer { // is associated with the passed Isolate in debug mode. inline v8::Local<v8::Object> object(v8::Isolate* isolate) const; - inline Persistent<v8::Object>& persistent(); + inline v8::Global<v8::Object>& persistent(); inline Environment* env() const; @@ -62,7 +61,7 @@ class BaseObject : public MemoryRetainer { template <typename T> static inline T* FromJSObject(v8::Local<v8::Object> object); - // Make the `Persistent` a weak reference and, `delete` this object once + // Make the `v8::Global` a weak reference and, `delete` this object once // the JS object has been garbage collected. inline void MakeWeak(); @@ -96,7 +95,7 @@ class BaseObject : public MemoryRetainer { friend int GenDebugSymbols(); friend class CleanupHookCallback; - Persistent<v8::Object> persistent_handle_; + v8::Global<v8::Object> persistent_handle_; Environment* env_; }; @@ -929,7 +929,7 @@ class Environment : public MemoryRetainer { std::unordered_map<uint32_t, contextify::ContextifyScript*> id_to_script_map; std::unordered_set<CompileFnEntry*> compile_fn_entries; - std::unordered_map<uint32_t, Persistent<v8::Function>> id_to_function_map; + std::unordered_map<uint32_t, v8::Global<v8::Function>> id_to_function_map; inline uint32_t get_next_module_id(); inline uint32_t get_next_script_id(); @@ -1292,7 +1292,7 @@ class Environment : public MemoryRetainer { template <typename T> void ForEachBaseObject(T&& iterator); -#define V(PropertyName, TypeName) Persistent<TypeName> PropertyName ## _; +#define V(PropertyName, TypeName) v8::Global<TypeName> PropertyName ## _; ENVIRONMENT_STRONG_PERSISTENT_VALUES(V) ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V) #undef V diff --git a/src/heap_utils.cc b/src/heap_utils.cc index d5b5afa4d5..654dfabafe 100644 --- a/src/heap_utils.cc +++ b/src/heap_utils.cc @@ -8,6 +8,7 @@ using v8::EmbedderGraph; using v8::EscapableHandleScope; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; +using v8::Global; using v8::HandleScope; using v8::HeapSnapshot; using v8::Isolate; @@ -56,7 +57,7 @@ class JSGraphJSNode : public EmbedderGraph::Node { }; private: - Persistent<Value> persistent_; + Global<Value> persistent_; }; class JSGraph : public EmbedderGraph { diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index 10b607ff6c..78af5508d0 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -34,6 +34,7 @@ using node::FatalError; using v8::Context; using v8::Function; +using v8::Global; using v8::HandleScope; using v8::Isolate; using v8::Local; @@ -834,7 +835,7 @@ void Agent::DisableAsyncHook() { } void Agent::ToggleAsyncHook(Isolate* isolate, - const node::Persistent<Function>& fn) { + const Global<Function>& fn) { CHECK(parent_env_->has_run_bootstrapping_code()); HandleScope handle_scope(isolate); CHECK(!fn.IsEmpty()); diff --git a/src/inspector_agent.h b/src/inspector_agent.h index b079ea675f..c7c92186d1 100644 --- a/src/inspector_agent.h +++ b/src/inspector_agent.h @@ -7,7 +7,6 @@ #endif #include "node_options-inl.h" -#include "node_persistent.h" #include "v8.h" #include <cstddef> @@ -114,7 +113,7 @@ class Agent { private: void ToggleAsyncHook(v8::Isolate* isolate, - const node::Persistent<v8::Function>& fn); + const v8::Global<v8::Function>& fn); node::Environment* parent_env_; // Encapsulates majority of the Inspector functionality @@ -133,8 +132,8 @@ class Agent { bool pending_enable_async_hook_ = false; bool pending_disable_async_hook_ = false; - node::Persistent<v8::Function> enable_async_hook_function_; - node::Persistent<v8::Function> disable_async_hook_function_; + v8::Global<v8::Function> enable_async_hook_function_; + v8::Global<v8::Function> disable_async_hook_function_; }; } // namespace inspector diff --git a/src/inspector_js_api.cc b/src/inspector_js_api.cc index 51ec14a63a..4948bd8797 100644 --- a/src/inspector_js_api.cc +++ b/src/inspector_js_api.cc @@ -15,6 +15,7 @@ using v8::Context; using v8::Function; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; +using v8::Global; using v8::HandleScope; using v8::Isolate; using v8::Local; @@ -116,7 +117,7 @@ class JSBindingsConnection : public AsyncWrap { private: std::unique_ptr<InspectorSession> session_; - Persistent<Function> callback_; + Global<Function> callback_; }; static bool InspectorEnabled(Environment* env) { diff --git a/src/js_native_api_v8_internals.h b/src/js_native_api_v8_internals.h index dcdc62297f..ddd219818c 100644 --- a/src/js_native_api_v8_internals.h +++ b/src/js_native_api_v8_internals.h @@ -29,7 +29,7 @@ namespace v8impl { template <typename T> -using Persistent = node::Persistent<T>; +using Persistent = v8::Global<T>; using PersistentToLocal = node::PersistentToLocal; diff --git a/src/memory_tracker-inl.h b/src/memory_tracker-inl.h index b17b8fbab2..da37f72c73 100644 --- a/src/memory_tracker-inl.h +++ b/src/memory_tracker-inl.h @@ -184,9 +184,9 @@ void MemoryTracker::TrackField(const char* edge_name, TrackField(edge_name, value.Get(isolate_)); } -template <typename T, typename Traits> +template <typename T> void MemoryTracker::TrackField(const char* edge_name, - const v8::Persistent<T, Traits>& value, + const v8::PersistentBase<T>& value, const char* node_name) { TrackField(edge_name, value.Get(isolate_)); } diff --git a/src/memory_tracker.h b/src/memory_tracker.h index 032c9a984e..d22116918a 100644 --- a/src/memory_tracker.h +++ b/src/memory_tracker.h @@ -90,9 +90,9 @@ class CleanupHookCallback; * NonPointerRetainerClass non_pointer_retainer; * InternalClass internal_member_; * std::vector<uv_async_t> vector_; - * node::Persistent<Object> target_; + * v8::Global<Object> target_; * - * node::Persistent<Object> wrapped_; + * v8::Global<Object> wrapped_; * } * * This creates the following graph: @@ -185,9 +185,9 @@ class MemoryTracker { void TrackField(const char* edge_name, const v8::Eternal<T>& value, const char* node_name); - template <typename T, typename Traits> + template <typename T> inline void TrackField(const char* edge_name, - const v8::Persistent<T, Traits>& value, + const v8::PersistentBase<T>& value, const char* node_name = nullptr); template <typename T> inline void TrackField(const char* edge_name, diff --git a/src/module_wrap.cc b/src/module_wrap.cc index d6ad0da622..a4e81dcc29 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -25,6 +25,7 @@ using v8::Context; using v8::Function; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; +using v8::Global; using v8::HandleScope; using v8::Integer; using v8::IntegrityLevel; @@ -611,7 +612,7 @@ Maybe<const PackageConfig*> GetPackageConfig(Environment* env, env->exports_string()).ToLocal(&exports_v) && (exports_v->IsObject() || exports_v->IsString() || exports_v->IsBoolean())) { - Persistent<Value> exports; + Global<Value> exports; exports.Reset(env->isolate(), exports_v); auto entry = env->package_json_cache.emplace(path, diff --git a/src/module_wrap.h b/src/module_wrap.h index fddb852a79..6c79781a9d 100644 --- a/src/module_wrap.h +++ b/src/module_wrap.h @@ -75,11 +75,11 @@ class ModuleWrap : public BaseObject { v8::Local<v8::Module> referrer); static ModuleWrap* GetFromModule(node::Environment*, v8::Local<v8::Module>); - Persistent<v8::Module> module_; - Persistent<v8::String> url_; + v8::Global<v8::Module> module_; + v8::Global<v8::String> url_; bool linked_ = false; - std::unordered_map<std::string, Persistent<v8::Promise>> resolve_cache_; - Persistent<v8::Context> context_; + std::unordered_map<std::string, v8::Global<v8::Promise>> resolve_cache_; + v8::Global<v8::Context> context_; uint32_t id_; }; diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 58175a8fd5..3b4be5a810 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -62,6 +62,7 @@ using v8::ArrayBufferView; using v8::Context; using v8::EscapableHandleScope; using v8::FunctionCallbackInfo; +using v8::Global; using v8::Integer; using v8::Isolate; using v8::Just; @@ -99,7 +100,7 @@ class CallbackInfo { FreeCallback callback, char* data, void* hint); - Persistent<ArrayBuffer> persistent_; + Global<ArrayBuffer> persistent_; FreeCallback const callback_; char* const data_; void* const hint_; diff --git a/src/node_contextify.h b/src/node_contextify.h index 4186e5190f..d04bf9ea28 100644 --- a/src/node_contextify.h +++ b/src/node_contextify.h @@ -100,7 +100,7 @@ class ContextifyContext { uint32_t index, const v8::PropertyCallbackInfo<v8::Boolean>& args); Environment* const env_; - Persistent<v8::Context> context_; + v8::Global<v8::Context> context_; }; class ContextifyScript : public BaseObject { @@ -129,7 +129,7 @@ class ContextifyScript : public BaseObject { inline uint32_t id() { return id_; } private: - node::Persistent<v8::UnboundScript> script_; + v8::Global<v8::UnboundScript> script_; uint32_t id_; }; diff --git a/src/node_crypto.h b/src/node_crypto.h index 657789afc4..44206b58dd 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -321,8 +321,8 @@ class SSLWrap { ClientHelloParser hello_parser_; - Persistent<v8::ArrayBufferView> ocsp_response_; - Persistent<v8::Value> sni_context_; + v8::Global<v8::ArrayBufferView> ocsp_response_; + v8::Global<v8::Value> sni_context_; friend class SecureContext; }; diff --git a/src/node_file.h b/src/node_file.h index f5c523fb07..b417d4916f 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -451,8 +451,8 @@ class FileHandle : public AsyncWrap, public StreamBase { CloseReq& operator=(const CloseReq&&) = delete; private: - Persistent<Promise> promise_{}; - Persistent<Value> ref_{}; + v8::Global<Promise> promise_{}; + v8::Global<Value> ref_{}; }; // Asynchronous close diff --git a/src/node_internals.h b/src/node_internals.h index fc924e3e16..198132d49d 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -28,7 +28,6 @@ #include "node.h" #include "node_binding.h" #include "node_mutex.h" -#include "node_persistent.h" #include "tracing/trace_event.h" #include "util-inl.h" #include "uv.h" diff --git a/src/node_persistent.h b/src/node_persistent.h deleted file mode 100644 index 6126ebc6c2..0000000000 --- a/src/node_persistent.h +++ /dev/null @@ -1,66 +0,0 @@ -#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>>; - -class PersistentToLocal { - public: - // If persistent.IsWeak() == false, then do not call persistent.Reset() - // while the returned Local<T> is still in scope, it will destroy the - // reference to the object. - template <class TypeName> - static inline v8::Local<TypeName> Default( - v8::Isolate* isolate, - const Persistent<TypeName>& persistent) { - if (persistent.IsWeak()) { - return PersistentToLocal::Weak(isolate, persistent); - } else { - return PersistentToLocal::Strong(persistent); - } - } - - // Unchecked conversion from a non-weak Persistent<T> to Local<T>, - // use with care! - // - // Do not call persistent.Reset() while the returned Local<T> is still in - // scope, it will destroy the reference to the object. - template <class TypeName> - static inline v8::Local<TypeName> Strong( - const Persistent<TypeName>& persistent) { - return *reinterpret_cast<v8::Local<TypeName>*>( - const_cast<Persistent<TypeName>*>(&persistent)); - } - - template <class TypeName> - static inline v8::Local<TypeName> Weak( - v8::Isolate* isolate, - const Persistent<TypeName>& persistent) { - return v8::Local<TypeName>::New(isolate, persistent); - } -}; - -} // namespace node - -#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS - -#endif // SRC_NODE_PERSISTENT_H_ diff --git a/src/node_util.cc b/src/node_util.cc index 961df0b73c..180c58c416 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -13,6 +13,7 @@ using v8::Context; using v8::Function; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; +using v8::Global; using v8::IndexFilter; using v8::Integer; using v8::Isolate; @@ -189,7 +190,7 @@ class WeakReference : public BaseObject { SET_NO_MEMORY_INFO() private: - Persistent<Object> target_; + Global<Object> target_; }; static void GuessHandleType(const FunctionCallbackInfo<Value>& args) { diff --git a/src/node_zlib.cc b/src/node_zlib.cc index b5abcfa585..f389257882 100644 --- a/src/node_zlib.cc +++ b/src/node_zlib.cc @@ -47,6 +47,7 @@ using v8::Context; using v8::Function; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; +using v8::Global; using v8::HandleScope; using v8::Int32; using v8::Integer; @@ -526,7 +527,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork { bool closed_ = false; unsigned int refs_ = 0; uint32_t* write_result_ = nullptr; - Persistent<Function> write_js_callback_; + Global<Function> write_js_callback_; std::atomic<ssize_t> unreported_allocations_{0}; size_t zlib_memory_ = 0; diff --git a/src/util.h b/src/util.h index 3a6cef2e35..c9c6056850 100644 --- a/src/util.h +++ b/src/util.h @@ -24,7 +24,6 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#include "node_persistent.h" #include "v8.h" #include <cassert> @@ -669,6 +668,44 @@ class SlicedArguments : public MaybeStackBuffer<v8::Local<v8::Value>> { const v8::FunctionCallbackInfo<v8::Value>& args, size_t start = 0); }; +// Convert a v8::PersistentBase, e.g. v8::Global, to a Local, with an extra +// optimization for strong persistent handles. +class PersistentToLocal { + public: + // If persistent.IsWeak() == false, then do not call persistent.Reset() + // while the returned Local<T> is still in scope, it will destroy the + // reference to the object. + template <class TypeName> + static inline v8::Local<TypeName> Default( + v8::Isolate* isolate, + const v8::PersistentBase<TypeName>& persistent) { + if (persistent.IsWeak()) { + return PersistentToLocal::Weak(isolate, persistent); + } else { + return PersistentToLocal::Strong(persistent); + } + } + + // Unchecked conversion from a non-weak Persistent<T> to Local<T>, + // use with care! + // + // Do not call persistent.Reset() while the returned Local<T> is still in + // scope, it will destroy the reference to the object. + template <class TypeName> + static inline v8::Local<TypeName> Strong( + const v8::PersistentBase<TypeName>& persistent) { + return *reinterpret_cast<v8::Local<TypeName>*>( + const_cast<v8::PersistentBase<TypeName>*>(&persistent)); + } + + template <class TypeName> + static inline v8::Local<TypeName> Weak( + v8::Isolate* isolate, + const v8::PersistentBase<TypeName>& persistent) { + return v8::Local<TypeName>::New(isolate, persistent); + } +}; + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS |