Skip to content

Commit

Permalink
src: make handle onclose property a Symbol
Browse files Browse the repository at this point in the history 
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]>
  • Loading branch information
@addaleax
addaleax committed Jun 6, 2018
1 parent 5c4d2e6 commit 2e886e9
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 5 deletions.
16 changes: 16 additions & 0 deletions src/async_wrap-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions src/async_wrap.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
20 changes: 19 additions & 1 deletion src/bootstrapper.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -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)
6 changes: 6 additions & 0 deletions src/env-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand All @@ -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) \
Expand Down
13 changes: 13 additions & 0 deletions src/env.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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, \
Expand Down
13 changes: 12 additions & 1 deletion src/env.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand All @@ -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") \
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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) \
Expand Down
8 changes: 5 additions & 3 deletions src/handle_wrap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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);
}
}

Expand Down
1 change: 1 addition & 0 deletions src/node_internals.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down

0 comments on commit 2e886e9

Please sign in to comment.