From 9a734132f9f92c1e398a3cce599cb74b94fa0b37 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 2 Jun 2018 13:13:59 +0200 Subject: [PATCH] src: make handle onclose property a Symbol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes the property “more” hidden when exposing a `HandleWrap` as public API, e.g. for upcoming `MessagePort`s. PR-URL: https://github.com/nodejs/node/pull/20876 Reviewed-By: Gireesh Punathil Reviewed-By: Benjamin Gruenbaum Reviewed-By: Shingo Inoue Reviewed-By: Matteo Collina Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: John-David Dalton Reviewed-By: Gus Caplan --- src/async_wrap-inl.h | 16 ++++++++++++++++ src/async_wrap.h | 8 ++++++++ src/bootstrapper.cc | 20 +++++++++++++++++++- src/env-inl.h | 6 ++++++ src/env.cc | 13 +++++++++++++ src/env.h | 13 ++++++++++++- src/handle_wrap.cc | 8 +++++--- src/node_internals.h | 1 + 8 files changed, 80 insertions(+), 5 deletions(-) diff --git a/src/async_wrap-inl.h b/src/async_wrap-inl.h index c9f12333243092..5763b17aa08bc4 100644 --- a/src/async_wrap-inl.h +++ b/src/async_wrap-inl.h @@ -65,6 +65,22 @@ inline v8::MaybeLocal AsyncWrap::MakeCallback( const v8::Local symbol, int argc, v8::Local* argv) { + return MakeCallback(symbol.As(), argc, argv); +} + + +inline v8::MaybeLocal AsyncWrap::MakeCallback( + const v8::Local symbol, + int argc, + v8::Local* argv) { + return MakeCallback(symbol.As(), argc, argv); +} + + +inline v8::MaybeLocal AsyncWrap::MakeCallback( + const v8::Local symbol, + int argc, + v8::Local* argv) { v8::Local cb_v = object()->Get(symbol); CHECK(cb_v->IsFunction()); return MakeCallback(cb_v.As(), argc, argv); diff --git a/src/async_wrap.h b/src/async_wrap.h index 451bcfe12e6717..377702a8d6ef9c 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -158,10 +158,18 @@ class AsyncWrap : public BaseObject { v8::MaybeLocal MakeCallback(const v8::Local cb, int argc, v8::Local* argv); + inline v8::MaybeLocal MakeCallback( + const v8::Local symbol, + int argc, + v8::Local* argv); inline v8::MaybeLocal MakeCallback( const v8::Local symbol, int argc, v8::Local* argv); + inline v8::MaybeLocal MakeCallback( + const v8::Local symbol, + int argc, + v8::Local* argv); inline v8::MaybeLocal MakeCallback(uint32_t index, int argc, v8::Local* argv); diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index 6c7c1af3e31cf6..35c7c4dc696ebd 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -17,6 +17,7 @@ using v8::Object; using v8::Promise; using v8::PromiseRejectEvent; using v8::PromiseRejectMessage; +using v8::String; using v8::Value; void SetupProcessObject(const FunctionCallbackInfo& args) { @@ -121,7 +122,7 @@ void SetupBootstrapObject(Environment* env, BOOTSTRAP_METHOD(_setgroups, SetGroups); #endif // __POSIX__ && !defined(__ANDROID__) && !defined(__CloudABI__) - auto should_abort_on_uncaught_toggle = + Local should_abort_on_uncaught_toggle = FIXED_ONE_BYTE_STRING(env->isolate(), "_shouldAbortOnUncaughtToggle"); CHECK(bootstrapper->Set(env->context(), should_abort_on_uncaught_toggle, @@ -130,4 +131,21 @@ void SetupBootstrapObject(Environment* env, } #undef BOOTSTRAP_METHOD +namespace symbols { + +void Initialize(Local target, + Local unused, + Local context) { + Environment* env = Environment::GetCurrent(context); +#define V(PropertyName, StringValue) \ + target->Set(env->context(), \ + env->PropertyName()->Name(), \ + env->PropertyName()).FromJust(); + PER_ISOLATE_SYMBOL_PROPERTIES(V) +#undef V +} + +} // namespace symbols } // namespace node + +NODE_MODULE_CONTEXT_AWARE_INTERNAL(symbols, node::symbols::Initialize) diff --git a/src/env-inl.h b/src/env-inl.h index cb8d0c4efe0405..aadb81271cf5d4 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -706,6 +706,7 @@ bool Environment::CleanupHookCallback::Equal::operator()( } #define VP(PropertyName, StringValue) V(v8::Private, PropertyName) +#define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName) #define VS(PropertyName, StringValue) V(v8::String, PropertyName) #define V(TypeName, PropertyName) \ inline \ @@ -714,21 +715,26 @@ bool Environment::CleanupHookCallback::Equal::operator()( return const_cast(this)->PropertyName ## _.Get(isolate); \ } PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(VP) + PER_ISOLATE_SYMBOL_PROPERTIES(VY) PER_ISOLATE_STRING_PROPERTIES(VS) #undef V #undef VS +#undef VY #undef VP #define VP(PropertyName, StringValue) V(v8::Private, PropertyName) +#define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName) #define VS(PropertyName, StringValue) V(v8::String, PropertyName) #define V(TypeName, PropertyName) \ inline v8::Local Environment::PropertyName() const { \ return isolate_data()->PropertyName(isolate()); \ } PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(VP) + PER_ISOLATE_SYMBOL_PROPERTIES(VY) PER_ISOLATE_STRING_PROPERTIES(VS) #undef V #undef VS +#undef VY #undef VP #define V(PropertyName, TypeName) \ diff --git a/src/env.cc b/src/env.cc index 7865ba95404df5..cb514828d2cdc4 100644 --- a/src/env.cc +++ b/src/env.cc @@ -23,6 +23,7 @@ using v8::Private; using v8::StackFrame; using v8::StackTrace; using v8::String; +using v8::Symbol; using v8::Value; IsolateData::IsolateData(Isolate* isolate, @@ -59,6 +60,18 @@ IsolateData::IsolateData(Isolate* isolate, sizeof(StringValue) - 1).ToLocalChecked())); PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(V) #undef V +#define V(PropertyName, StringValue) \ + PropertyName ## _.Set( \ + isolate, \ + Symbol::New( \ + isolate, \ + String::NewFromOneByte( \ + isolate, \ + reinterpret_cast(StringValue), \ + v8::NewStringType::kInternalized, \ + sizeof(StringValue) - 1).ToLocalChecked())); + PER_ISOLATE_SYMBOL_PROPERTIES(V) +#undef V #define V(PropertyName, StringValue) \ PropertyName ## _.Set( \ isolate, \ diff --git a/src/env.h b/src/env.h index 96252fa12a1397..7a432eaa3d4ff0 100644 --- a/src/env.h +++ b/src/env.h @@ -107,6 +107,11 @@ struct PackageConfig { V(napi_env, "node:napi:env") \ V(napi_wrapper, "node:napi:wrapper") \ +// Symbols are per-isolate primitives but Environment proxies them +// for the sake of convenience. +#define PER_ISOLATE_SYMBOL_PROPERTIES(V) \ + V(handle_onclose_symbol, "handle_onclose") \ + // Strings are per-isolate primitives but Environment proxies them // for the sake of convenience. Strings should be ASCII-only. #define PER_ISOLATE_STRING_PROPERTIES(V) \ @@ -127,7 +132,6 @@ struct PackageConfig { V(chunks_sent_since_last_write_string, "chunksSentSinceLastWrite") \ V(constants_string, "constants") \ V(oncertcb_string, "oncertcb") \ - V(onclose_string, "_onclose") \ V(code_string, "code") \ V(cwd_string, "cwd") \ V(dest_string, "dest") \ @@ -356,10 +360,12 @@ class IsolateData { inline MultiIsolatePlatform* platform() const; #define VP(PropertyName, StringValue) V(v8::Private, PropertyName) +#define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName) #define VS(PropertyName, StringValue) V(v8::String, PropertyName) #define V(TypeName, PropertyName) \ inline v8::Local PropertyName(v8::Isolate* isolate) const; PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(VP) + PER_ISOLATE_SYMBOL_PROPERTIES(VY) PER_ISOLATE_STRING_PROPERTIES(VS) #undef V #undef VS @@ -370,10 +376,12 @@ class IsolateData { private: #define VP(PropertyName, StringValue) V(v8::Private, PropertyName) +#define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName) #define VS(PropertyName, StringValue) V(v8::String, PropertyName) #define V(TypeName, PropertyName) \ v8::Eternal PropertyName ## _; PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(VP) + PER_ISOLATE_SYMBOL_PROPERTIES(VY) PER_ISOLATE_STRING_PROPERTIES(VS) #undef V #undef VS @@ -737,13 +745,16 @@ class Environment { // Strings and private symbols are shared across shared contexts // The getters simply proxy to the per-isolate primitive. #define VP(PropertyName, StringValue) V(v8::Private, PropertyName) +#define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName) #define VS(PropertyName, StringValue) V(v8::String, PropertyName) #define V(TypeName, PropertyName) \ inline v8::Local PropertyName() const; PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(VP) + PER_ISOLATE_SYMBOL_PROPERTIES(VY) PER_ISOLATE_STRING_PROPERTIES(VS) #undef V #undef VS +#undef VY #undef VP #define V(PropertyName, TypeName) \ diff --git a/src/handle_wrap.cc b/src/handle_wrap.cc index bbc124ff69dc89..4c2a33aa84459d 100644 --- a/src/handle_wrap.cc +++ b/src/handle_wrap.cc @@ -76,7 +76,9 @@ void HandleWrap::Close(Local close_callback) { state_ = kClosing; if (!close_callback.IsEmpty() && close_callback->IsFunction()) { - object()->Set(env()->context(), env()->onclose_string(), close_callback) + object()->Set(env()->context(), + env()->handle_onclose_symbol(), + close_callback) .FromMaybe(false); } } @@ -121,9 +123,9 @@ void HandleWrap::OnClose(uv_handle_t* handle) { wrap->OnClose(); - if (wrap->object()->Has(env->context(), env->onclose_string()) + if (wrap->object()->Has(env->context(), env->handle_onclose_symbol()) .FromMaybe(false)) { - wrap->MakeCallback(env->onclose_string(), 0, nullptr); + wrap->MakeCallback(env->handle_onclose_symbol(), 0, nullptr); } } diff --git a/src/node_internals.h b/src/node_internals.h index 3014a0e5f7a442..7bf4eaf1ecf86a 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -125,6 +125,7 @@ struct sockaddr; V(stream_pipe) \ V(stream_wrap) \ V(string_decoder) \ + V(symbols) \ V(tcp_wrap) \ V(timer_wrap) \ V(trace_events) \