src: make handle onclose property a Symbol

This makes the property “more” hidden when exposing a `HandleWrap` as public API, e.g. for upcoming `MessagePort`s. PR-URL: #20876 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Shingo Inoue <leko.noor@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: John-David Dalton <john.david.dalton@gmail.com> Reviewed-By: Gus Caplan <me@gus.host>
nodejs · Jun 2, 2018 · 9a734132f9f92c1e398a3cce599cb74b94fa0b37 · 9a73413
1 parent e6f0680
commit 9a734132f9f92c1e398a3cce599cb74b94fa0b37
Unified Split

Showing with 80 additions and 5 deletions.

+16 −0 src/async_wrap-inl.h

+8 −0 src/async_wrap.h

+19 −1 src/bootstrapper.cc

+6 −0 src/env-inl.h

+13 −0 src/env.cc

+12 −1 src/env.h

+5 −3 src/handle_wrap.cc

+1 −0 src/node_internals.h
diff --git a/src/async_wrap-inl.h b/src/async_wrap-inl.h
@@ -65,6 +65,22 @@ inline v8::MaybeLocal<v8::Value> AsyncWrap::MakeCallback(
    const v8::Local<v8::String> symbol,
    int argc,
    v8::Local<v8::Value>* argv) {
+  
+  return MakeCallback(symbol.As<v8::Name>(), argc, argv);
+  
+}
+  
+
+  
+
+  
+inline v8::MaybeLocal<v8::Value> AsyncWrap::MakeCallback(
+  
+    const v8::Local<v8::Symbol> symbol,
+  
+    int argc,
+  
+    v8::Local<v8::Value>* argv) {
+  
+  return MakeCallback(symbol.As<v8::Name>(), argc, argv);
+  
+}
+  
+
+  
+
+  
+inline v8::MaybeLocal<v8::Value> AsyncWrap::MakeCallback(
+  
+    const v8::Local<v8::Name> symbol,
+  
+    int argc,
+  
+    v8::Local<v8::Value>* argv) {
  v8::Local<v8::Value> cb_v = object()->Get(symbol);
  CHECK(cb_v->IsFunction());
  return MakeCallback(cb_v.As<v8::Function>(), argc, argv);

diff --git a/src/async_wrap.h b/src/async_wrap.h
@@ -158,10 +158,18 @@ class AsyncWrap : public BaseObject {
  v8::MaybeLocal<v8::Value> MakeCallback(const v8::Local<v8::Function> cb,
                                         int argc,
                                         v8::Local<v8::Value>* argv);
+  
+  inline v8::MaybeLocal<v8::Value> MakeCallback(
+  
+      const v8::Local<v8::Symbol> symbol,
+  
+      int argc,
+  
+      v8::Local<v8::Value>* argv);
  inline v8::MaybeLocal<v8::Value> MakeCallback(
      const v8::Local<v8::String> symbol,
      int argc,
      v8::Local<v8::Value>* argv);
+  
+  inline v8::MaybeLocal<v8::Value> MakeCallback(
+  
+      const v8::Local<v8::Name> symbol,
+  
+      int argc,
+  
+      v8::Local<v8::Value>* argv);
  inline v8::MaybeLocal<v8::Value> MakeCallback(uint32_t index,
                                                int argc,
                                                v8::Local<v8::Value>* argv);

diff --git 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<Value>& 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<String> 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<Object> target,
+  
+                Local<Value> unused,
+  
+                Local<Context> 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
@@ -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<IsolateData*>(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<TypeName> 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
@@ -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<const uint8_t*>(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
@@ -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<TypeName> 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<TypeName> 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<TypeName> 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
@@ -76,7 +76,9 @@ void HandleWrap::Close(Local<Value> 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
@@ -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)                                                           \