Skip to content

Commit 9a73413

Browse files
addaleaxtargos
addaleax
authored and
targos
committed
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]>
1 parent e6f0680 commit 9a73413

File tree

8 files changed

+80
-5
lines changed

8 files changed

+80
-5
lines changed

‎src/async_wrap-inl.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,22 @@ inline v8::MaybeLocal<v8::Value> AsyncWrap::MakeCallback(
6565
const v8::Local<v8::String> symbol,
6666
int argc,
6767
v8::Local<v8::Value>* argv) {
68+
return MakeCallback(symbol.As<v8::Name>(), argc, argv);
69+
}
70+
71+
72+
inline v8::MaybeLocal<v8::Value> AsyncWrap::MakeCallback(
73+
const v8::Local<v8::Symbol> symbol,
74+
int argc,
75+
v8::Local<v8::Value>* argv) {
76+
return MakeCallback(symbol.As<v8::Name>(), argc, argv);
77+
}
78+
79+
80+
inline v8::MaybeLocal<v8::Value> AsyncWrap::MakeCallback(
81+
const v8::Local<v8::Name> symbol,
82+
int argc,
83+
v8::Local<v8::Value>* argv) {
6884
v8::Local<v8::Value> cb_v = object()->Get(symbol);
6985
CHECK(cb_v->IsFunction());
7086
return MakeCallback(cb_v.As<v8::Function>(), argc, argv);

‎src/async_wrap.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,10 +158,18 @@ class AsyncWrap : public BaseObject {
158158
v8::MaybeLocal<v8::Value> MakeCallback(const v8::Local<v8::Function> cb,
159159
int argc,
160160
v8::Local<v8::Value>* argv);
161+
inline v8::MaybeLocal<v8::Value> MakeCallback(
162+
const v8::Local<v8::Symbol> symbol,
163+
int argc,
164+
v8::Local<v8::Value>* argv);
161165
inline v8::MaybeLocal<v8::Value> MakeCallback(
162166
const v8::Local<v8::String> symbol,
163167
int argc,
164168
v8::Local<v8::Value>* argv);
169+
inline v8::MaybeLocal<v8::Value> MakeCallback(
170+
const v8::Local<v8::Name> symbol,
171+
int argc,
172+
v8::Local<v8::Value>* argv);
165173
inline v8::MaybeLocal<v8::Value> MakeCallback(uint32_t index,
166174
int argc,
167175
v8::Local<v8::Value>* argv);

‎src/bootstrapper.cc

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ using v8::Object;
1717
using v8::Promise;
1818
using v8::PromiseRejectEvent;
1919
using v8::PromiseRejectMessage;
20+
using v8::String;
2021
using v8::Value;
2122

2223
void SetupProcessObject(const FunctionCallbackInfo<Value>& args) {
@@ -121,7 +122,7 @@ void SetupBootstrapObject(Environment* env,
121122
BOOTSTRAP_METHOD(_setgroups, SetGroups);
122123
#endif // __POSIX__ && !defined(__ANDROID__) && !defined(__CloudABI__)
123124

124-
auto should_abort_on_uncaught_toggle =
125+
Local<String> should_abort_on_uncaught_toggle =
125126
FIXED_ONE_BYTE_STRING(env->isolate(), "_shouldAbortOnUncaughtToggle");
126127
CHECK(bootstrapper->Set(env->context(),
127128
should_abort_on_uncaught_toggle,
@@ -130,4 +131,21 @@ void SetupBootstrapObject(Environment* env,
130131
}
131132
#undef BOOTSTRAP_METHOD
132133

134+
namespace symbols {
135+
136+
void Initialize(Local<Object> target,
137+
Local<Value> unused,
138+
Local<Context> context) {
139+
Environment* env = Environment::GetCurrent(context);
140+
#define V(PropertyName, StringValue) \
141+
target->Set(env->context(), \
142+
env->PropertyName()->Name(), \
143+
env->PropertyName()).FromJust();
144+
PER_ISOLATE_SYMBOL_PROPERTIES(V)
145+
#undef V
146+
}
147+
148+
} // namespace symbols
133149
} // namespace node
150+
151+
NODE_MODULE_CONTEXT_AWARE_INTERNAL(symbols, node::symbols::Initialize)

‎src/env-inl.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -706,6 +706,7 @@ bool Environment::CleanupHookCallback::Equal::operator()(
706706
}
707707

708708
#define VP(PropertyName, StringValue) V(v8::Private, PropertyName)
709+
#define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
709710
#define VS(PropertyName, StringValue) V(v8::String, PropertyName)
710711
#define V(TypeName, PropertyName) \
711712
inline \
@@ -714,21 +715,26 @@ bool Environment::CleanupHookCallback::Equal::operator()(
714715
return const_cast<IsolateData*>(this)->PropertyName ## _.Get(isolate); \
715716
}
716717
PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(VP)
718+
PER_ISOLATE_SYMBOL_PROPERTIES(VY)
717719
PER_ISOLATE_STRING_PROPERTIES(VS)
718720
#undef V
719721
#undef VS
722+
#undef VY
720723
#undef VP
721724

722725
#define VP(PropertyName, StringValue) V(v8::Private, PropertyName)
726+
#define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
723727
#define VS(PropertyName, StringValue) V(v8::String, PropertyName)
724728
#define V(TypeName, PropertyName) \
725729
inline v8::Local<TypeName> Environment::PropertyName() const { \
726730
return isolate_data()->PropertyName(isolate()); \
727731
}
728732
PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(VP)
733+
PER_ISOLATE_SYMBOL_PROPERTIES(VY)
729734
PER_ISOLATE_STRING_PROPERTIES(VS)
730735
#undef V
731736
#undef VS
737+
#undef VY
732738
#undef VP
733739

734740
#define V(PropertyName, TypeName) \

‎src/env.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ using v8::Private;
2323
using v8::StackFrame;
2424
using v8::StackTrace;
2525
using v8::String;
26+
using v8::Symbol;
2627
using v8::Value;
2728

2829
IsolateData::IsolateData(Isolate* isolate,
@@ -59,6 +60,18 @@ IsolateData::IsolateData(Isolate* isolate,
5960
sizeof(StringValue) - 1).ToLocalChecked()));
6061
PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(V)
6162
#undef V
63+
#define V(PropertyName, StringValue) \
64+
PropertyName ## _.Set( \
65+
isolate, \
66+
Symbol::New( \
67+
isolate, \
68+
String::NewFromOneByte( \
69+
isolate, \
70+
reinterpret_cast<const uint8_t*>(StringValue), \
71+
v8::NewStringType::kInternalized, \
72+
sizeof(StringValue) - 1).ToLocalChecked()));
73+
PER_ISOLATE_SYMBOL_PROPERTIES(V)
74+
#undef V
6275
#define V(PropertyName, StringValue) \
6376
PropertyName ## _.Set( \
6477
isolate, \

‎src/env.h

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,11 @@ struct PackageConfig {
107107
V(napi_env, "node:napi:env") \
108108
V(napi_wrapper, "node:napi:wrapper") \
109109

110+
// Symbols are per-isolate primitives but Environment proxies them
111+
// for the sake of convenience.
112+
#define PER_ISOLATE_SYMBOL_PROPERTIES(V) \
113+
V(handle_onclose_symbol, "handle_onclose") \
114+
110115
// Strings are per-isolate primitives but Environment proxies them
111116
// for the sake of convenience. Strings should be ASCII-only.
112117
#define PER_ISOLATE_STRING_PROPERTIES(V) \
@@ -127,7 +132,6 @@ struct PackageConfig {
127132
V(chunks_sent_since_last_write_string, "chunksSentSinceLastWrite") \
128133
V(constants_string, "constants") \
129134
V(oncertcb_string, "oncertcb") \
130-
V(onclose_string, "_onclose") \
131135
V(code_string, "code") \
132136
V(cwd_string, "cwd") \
133137
V(dest_string, "dest") \
@@ -356,10 +360,12 @@ class IsolateData {
356360
inline MultiIsolatePlatform* platform() const;
357361

358362
#define VP(PropertyName, StringValue) V(v8::Private, PropertyName)
363+
#define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
359364
#define VS(PropertyName, StringValue) V(v8::String, PropertyName)
360365
#define V(TypeName, PropertyName) \
361366
inline v8::Local<TypeName> PropertyName(v8::Isolate* isolate) const;
362367
PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(VP)
368+
PER_ISOLATE_SYMBOL_PROPERTIES(VY)
363369
PER_ISOLATE_STRING_PROPERTIES(VS)
364370
#undef V
365371
#undef VS
@@ -370,10 +376,12 @@ class IsolateData {
370376

371377
private:
372378
#define VP(PropertyName, StringValue) V(v8::Private, PropertyName)
379+
#define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
373380
#define VS(PropertyName, StringValue) V(v8::String, PropertyName)
374381
#define V(TypeName, PropertyName) \
375382
v8::Eternal<TypeName> PropertyName ## _;
376383
PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(VP)
384+
PER_ISOLATE_SYMBOL_PROPERTIES(VY)
377385
PER_ISOLATE_STRING_PROPERTIES(VS)
378386
#undef V
379387
#undef VS
@@ -737,13 +745,16 @@ class Environment {
737745
// Strings and private symbols are shared across shared contexts
738746
// The getters simply proxy to the per-isolate primitive.
739747
#define VP(PropertyName, StringValue) V(v8::Private, PropertyName)
748+
#define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
740749
#define VS(PropertyName, StringValue) V(v8::String, PropertyName)
741750
#define V(TypeName, PropertyName) \
742751
inline v8::Local<TypeName> PropertyName() const;
743752
PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(VP)
753+
PER_ISOLATE_SYMBOL_PROPERTIES(VY)
744754
PER_ISOLATE_STRING_PROPERTIES(VS)
745755
#undef V
746756
#undef VS
757+
#undef VY
747758
#undef VP
748759

749760
#define V(PropertyName, TypeName) \

‎src/handle_wrap.cc

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,9 @@ void HandleWrap::Close(Local<Value> close_callback) {
7676
state_ = kClosing;
7777

7878
if (!close_callback.IsEmpty() && close_callback->IsFunction()) {
79-
object()->Set(env()->context(), env()->onclose_string(), close_callback)
79+
object()->Set(env()->context(),
80+
env()->handle_onclose_symbol(),
81+
close_callback)
8082
.FromMaybe(false);
8183
}
8284
}
@@ -121,9 +123,9 @@ void HandleWrap::OnClose(uv_handle_t* handle) {
121123

122124
wrap->OnClose();
123125

124-
if (wrap->object()->Has(env->context(), env->onclose_string())
126+
if (wrap->object()->Has(env->context(), env->handle_onclose_symbol())
125127
.FromMaybe(false)) {
126-
wrap->MakeCallback(env->onclose_string(), 0, nullptr);
128+
wrap->MakeCallback(env->handle_onclose_symbol(), 0, nullptr);
127129
}
128130
}
129131

‎src/node_internals.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ struct sockaddr;
125125
V(stream_pipe) \
126126
V(stream_wrap) \
127127
V(string_decoder) \
128+
V(symbols) \
128129
V(tcp_wrap) \
129130
V(timer_wrap) \
130131
V(trace_events) \

0 commit comments

Comments
 (0)