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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Shingo Inoue <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: John-David Dalton <[email protected]> Reviewed-By: Gus Caplan <[email protected]>

Author: addaleax

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){
+returnMakeCallback(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){
+returnMakeCallback(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());
 returnMakeCallback(cb_v.As<v8::Function>(), argc, argv);

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);

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;
 
 voidSetupProcessObject(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
 
+namespacesymbols{
+
+voidInitialize(Local<Object> target,
+ Local<Value> unused,
+ Local<Context> context){
+ Environment* env = Environment::GetCurrent(context);
+#defineV(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)

src/env-inl.h

@@ -706,6 +706,7 @@ bool Environment::CleanupHookCallback::Equal::operator()(
 }
 
 #defineVP(PropertyName, StringValue) V(v8::Private, PropertyName)
+#defineVY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
 #defineVS(PropertyName, StringValue) V(v8::String, PropertyName)
 #defineV(TypeName, PropertyName) \
 inline \
@@ -714,21 +715,26 @@ bool Environment::CleanupHookCallback::Equal::operator()(
 returnconst_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
 
 #defineVP(PropertyName, StringValue) V(v8::Private, PropertyName)
+#defineVY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
 #defineVS(PropertyName, StringValue) V(v8::String, PropertyName)
 #defineV(TypeName, PropertyName) \
 inline v8::Local<TypeName> Environment::PropertyName() const{\
 returnisolate_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
 
 #defineV(PropertyName, TypeName) \

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
+#defineV(PropertyName, StringValue) \
+ PropertyName ## _.Set( \
+ isolate, \
+Symbol::New( \
+ isolate, \
+String::NewFromOneByte( \
+ isolate, \
+reinterpret_cast<constuint8_t*>(StringValue), \
+ v8::NewStringType::kInternalized, \
+sizeof(StringValue) - 1).ToLocalChecked()));
+PER_ISOLATE_SYMBOL_PROPERTIES(V)
+#undef V
 #defineV(PropertyName, StringValue) \
  PropertyName ## _.Set( \
  isolate, \

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.
+#definePER_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.
 #definePER_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;
 
 #defineVP(PropertyName, StringValue) V(v8::Private, PropertyName)
+#defineVY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
 #defineVS(PropertyName, StringValue) V(v8::String, PropertyName)
 #defineV(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:
 #defineVP(PropertyName, StringValue) V(v8::Private, PropertyName)
+#defineVY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
 #defineVS(PropertyName, StringValue) V(v8::String, PropertyName)
 #defineV(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.
 #defineVP(PropertyName, StringValue) V(v8::Private, PropertyName)
+#defineVY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
 #defineVS(PropertyName, StringValue) V(v8::String, PropertyName)
 #defineV(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
 
 #defineV(PropertyName, TypeName) \

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);
  }
 }
 

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) \