From b2eeb79107b7d3e40017c5fa456b57726716e858 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 23 Sep 2018 19:24:33 +0200 Subject: [PATCH 1/3] src: use JS inheritance for `AsyncWrap` For all classes descending from `AsyncWrap`, use JS inheritance instead of manually adding methods to the individual classes. This allows cleanup of some code around transferring handles over IPC. --- lib/internal/worker.js | 3 +- node.gyp | 1 - src/async_wrap.cc | 23 +++++--- src/async_wrap.h | 10 +--- src/cares_wrap.cc | 8 +-- src/env.h | 5 +- src/fs_event_wrap.cc | 2 +- src/handle_wrap.cc | 20 ++++--- src/handle_wrap.h | 4 +- src/inspector_js_api.cc | 2 +- src/js_stream.cc | 3 +- src/node_file.cc | 10 ++-- src/node_http2.cc | 8 +-- src/node_http_parser.cc | 2 +- src/node_messaging.cc | 4 +- src/node_stat_watcher.cc | 4 +- src/node_worker.cc | 2 +- src/node_wrap.h | 72 ----------------------- src/node_zlib.cc | 2 +- src/pipe_wrap.cc | 7 +-- src/process_wrap.cc | 34 +++++------ src/signal_wrap.cc | 4 +- src/stream_pipe.cc | 2 +- src/stream_wrap.cc | 63 ++++++++++++-------- src/stream_wrap.h | 4 +- src/tcp_wrap.cc | 6 +- src/tls_wrap.cc | 2 +- src/tty_wrap.cc | 6 +- src/udp_wrap.cc | 5 +- test/parallel/test-accessor-properties.js | 10 ++-- 30 files changed, 130 insertions(+), 198 deletions(-) delete mode 100644 src/node_wrap.h diff --git a/lib/internal/worker.js b/lib/internal/worker.js index ceec8469b19a9a..2cffb77e521978 100644 --- a/lib/internal/worker.js +++ b/lib/internal/worker.js @@ -73,7 +73,8 @@ Object.setPrototypeOf(MessagePort.prototype, EventEmitter.prototype); delete MessagePort.prototype.stop; delete MessagePort.prototype.drain; delete MessagePort.prototype.hasRef; -delete MessagePort.prototype.getAsyncId; +MessagePort.prototype.ref = MessagePortPrototype.ref; +MessagePort.prototype.unref = MessagePortPrototype.unref; // A communication channel consisting of a handle (that wraps around an // uv_async_t) which can receive information from other threads and emits diff --git a/node.gyp b/node.gyp index e1adf8cd12e30b..72617a463c2ef0 100644 --- a/node.gyp +++ b/node.gyp @@ -427,7 +427,6 @@ 'src/node_root_certs.h', 'src/node_version.h', 'src/node_watchdog.h', - 'src/node_wrap.h', 'src/node_revert.h', 'src/node_i18n.h', 'src/node_worker.h', diff --git a/src/async_wrap.cc b/src/async_wrap.cc index b04ab6808704b1..f9e2beaf97338f 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -64,7 +64,8 @@ struct AsyncWrapObject : public AsyncWrap { static inline void New(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK(args.IsConstructCall()); - CHECK(env->async_wrap_constructor_template()->HasInstance(args.This())); + CHECK(env->async_wrap_object_constructor_template()->HasInstance( + args.This())); CHECK(args[0]->IsUint32()); auto type = static_cast(args[0].As()->Value()); new AsyncWrapObject(env, args.This(), type); @@ -424,12 +425,16 @@ void AsyncWrap::QueueDestroyAsyncId(const FunctionCallbackInfo& args) { args[0].As()->Value()); } -void AsyncWrap::AddWrapMethods(Environment* env, - Local constructor, - int flag) { - env->SetProtoMethod(constructor, "getAsyncId", AsyncWrap::GetAsyncId); - if (flag & kFlagHasReset) - env->SetProtoMethod(constructor, "asyncReset", AsyncWrap::AsyncReset); +Local AsyncWrap::GetConstructorTemplate(Environment* env) { + Local tmpl = env->async_wrap_ctor_template(); + if (tmpl.IsEmpty()) { + tmpl = env->NewFunctionTemplate(nullptr); + tmpl->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "AsyncWrap")); + env->SetProtoMethod(tmpl, "getAsyncId", AsyncWrap::GetAsyncId); + env->SetProtoMethod(tmpl, "asyncReset", AsyncWrap::AsyncReset); + env->set_async_wrap_ctor_template(tmpl); + } + return tmpl; } void AsyncWrap::Initialize(Local target, @@ -529,13 +534,13 @@ void AsyncWrap::Initialize(Local target, auto class_name = FIXED_ONE_BYTE_STRING(env->isolate(), "AsyncWrap"); auto function_template = env->NewFunctionTemplate(AsyncWrapObject::New); function_template->SetClassName(class_name); - AsyncWrap::AddWrapMethods(env, function_template); + function_template->Inherit(AsyncWrap::GetConstructorTemplate(env)); auto instance_template = function_template->InstanceTemplate(); instance_template->SetInternalFieldCount(1); auto function = function_template->GetFunction(env->context()).ToLocalChecked(); target->Set(env->context(), class_name, function).FromJust(); - env->set_async_wrap_constructor_template(function_template); + env->set_async_wrap_object_constructor_template(function_template); } } diff --git a/src/async_wrap.h b/src/async_wrap.h index 50864837d62787..360380afc3459b 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -104,11 +104,6 @@ class AsyncWrap : public BaseObject { PROVIDERS_LENGTH, }; - enum Flags { - kFlagNone = 0x0, - kFlagHasReset = 0x1 - }; - AsyncWrap(Environment* env, v8::Local object, ProviderType provider, @@ -116,9 +111,8 @@ class AsyncWrap : public BaseObject { virtual ~AsyncWrap(); - static void AddWrapMethods(Environment* env, - v8::Local constructor, - int flags = kFlagNone); + static v8::Local GetConstructorTemplate( + Environment* env); static void Initialize(v8::Local target, v8::Local unused, diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index a967414aecdec2..3187b7d4760b1d 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -2220,7 +2220,7 @@ void Initialize(Local target, Local aiw = BaseObject::MakeLazilyInitializedJSTemplate(env); - AsyncWrap::AddWrapMethods(env, aiw); + aiw->Inherit(AsyncWrap::GetConstructorTemplate(env)); Local addrInfoWrapString = FIXED_ONE_BYTE_STRING(env->isolate(), "GetAddrInfoReqWrap"); aiw->SetClassName(addrInfoWrapString); @@ -2228,7 +2228,7 @@ void Initialize(Local target, Local niw = BaseObject::MakeLazilyInitializedJSTemplate(env); - AsyncWrap::AddWrapMethods(env, niw); + niw->Inherit(AsyncWrap::GetConstructorTemplate(env)); Local nameInfoWrapString = FIXED_ONE_BYTE_STRING(env->isolate(), "GetNameInfoReqWrap"); niw->SetClassName(nameInfoWrapString); @@ -2236,7 +2236,7 @@ void Initialize(Local target, Local qrw = BaseObject::MakeLazilyInitializedJSTemplate(env); - AsyncWrap::AddWrapMethods(env, qrw); + qrw->Inherit(AsyncWrap::GetConstructorTemplate(env)); Local queryWrapString = FIXED_ONE_BYTE_STRING(env->isolate(), "QueryReqWrap"); qrw->SetClassName(queryWrapString); @@ -2245,7 +2245,7 @@ void Initialize(Local target, Local channel_wrap = env->NewFunctionTemplate(ChannelWrap::New); channel_wrap->InstanceTemplate()->SetInternalFieldCount(1); - AsyncWrap::AddWrapMethods(env, channel_wrap); + channel_wrap->Inherit(AsyncWrap::GetConstructorTemplate(env)); env->SetProtoMethod(channel_wrap, "queryAny", Query); env->SetProtoMethod(channel_wrap, "queryA", Query); diff --git a/src/env.h b/src/env.h index feeeda661cec66..a4a7a2e8a2b631 100644 --- a/src/env.h +++ b/src/env.h @@ -319,7 +319,8 @@ struct PackageConfig { V(async_hooks_destroy_function, v8::Function) \ V(async_hooks_init_function, v8::Function) \ V(async_hooks_promise_resolve_function, v8::Function) \ - V(async_wrap_constructor_template, v8::FunctionTemplate) \ + V(async_wrap_object_constructor_template, v8::FunctionTemplate) \ + V(async_wrap_ctor_template, v8::FunctionTemplate) \ V(buffer_prototype_object, v8::Object) \ V(context, v8::Context) \ V(domain_callback, v8::Function) \ @@ -329,6 +330,7 @@ struct PackageConfig { V(filehandlereadwrap_template, v8::ObjectTemplate) \ V(fsreqpromise_constructor_template, v8::ObjectTemplate) \ V(fs_use_promises_symbol, v8::Symbol) \ + V(handle_wrap_ctor_template, v8::FunctionTemplate) \ V(host_import_module_dynamically_callback, v8::Function) \ V(host_initialize_import_meta_object_callback, v8::Function) \ V(http2ping_constructor_template, v8::ObjectTemplate) \ @@ -336,6 +338,7 @@ struct PackageConfig { V(http2stream_constructor_template, v8::ObjectTemplate) \ V(immediate_callback_function, v8::Function) \ V(inspector_console_api_object, v8::Object) \ + V(libuv_stream_wrap_ctor_template, v8::FunctionTemplate) \ V(message_port, v8::Object) \ V(message_port_constructor_template, v8::FunctionTemplate) \ V(pipe_constructor_template, v8::FunctionTemplate) \ diff --git a/src/fs_event_wrap.cc b/src/fs_event_wrap.cc index c51054819f7c01..aaf03dcb2b5588 100644 --- a/src/fs_event_wrap.cc +++ b/src/fs_event_wrap.cc @@ -105,7 +105,7 @@ void FSEventWrap::Initialize(Local target, t->InstanceTemplate()->SetInternalFieldCount(1); t->SetClassName(fsevent_string); - AsyncWrap::AddWrapMethods(env, t); + t->Inherit(AsyncWrap::GetConstructorTemplate(env)); env->SetProtoMethod(t, "start", Start); env->SetProtoMethod(t, "close", Close); diff --git a/src/handle_wrap.cc b/src/handle_wrap.cc index 9281300146c4f3..d4c5962c35e806 100644 --- a/src/handle_wrap.cc +++ b/src/handle_wrap.cc @@ -130,13 +130,19 @@ void HandleWrap::OnClose(uv_handle_t* handle) { } } - -void HandleWrap::AddWrapMethods(Environment* env, - Local t) { - env->SetProtoMethod(t, "close", HandleWrap::Close); - env->SetProtoMethodNoSideEffect(t, "hasRef", HandleWrap::HasRef); - env->SetProtoMethod(t, "ref", HandleWrap::Ref); - env->SetProtoMethod(t, "unref", HandleWrap::Unref); +Local HandleWrap::GetConstructorTemplate(Environment* env) { + Local tmpl = env->handle_wrap_ctor_template(); + if (tmpl.IsEmpty()) { + tmpl = env->NewFunctionTemplate(nullptr); + tmpl->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "HandleWrap")); + tmpl->Inherit(AsyncWrap::GetConstructorTemplate(env)); + env->SetProtoMethod(tmpl, "close", HandleWrap::Close); + env->SetProtoMethodNoSideEffect(tmpl, "hasRef", HandleWrap::HasRef); + env->SetProtoMethod(tmpl, "ref", HandleWrap::Ref); + env->SetProtoMethod(tmpl, "unref", HandleWrap::Unref); + env->set_handle_wrap_ctor_template(tmpl); + } + return tmpl; } diff --git a/src/handle_wrap.h b/src/handle_wrap.h index 443d28bf523933..b2722511c3c09f 100644 --- a/src/handle_wrap.h +++ b/src/handle_wrap.h @@ -73,8 +73,8 @@ class HandleWrap : public AsyncWrap { virtual void Close( v8::Local close_callback = v8::Local()); - static void AddWrapMethods(Environment* env, - v8::Local constructor); + static v8::Local GetConstructorTemplate( + Environment* env); protected: HandleWrap(Environment* env, diff --git a/src/inspector_js_api.cc b/src/inspector_js_api.cc index 52184111f5527a..49e1dcc6e8a30c 100644 --- a/src/inspector_js_api.cc +++ b/src/inspector_js_api.cc @@ -307,7 +307,7 @@ void Initialize(Local target, Local unused, env->NewFunctionTemplate(JSBindingsConnection::New); tmpl->InstanceTemplate()->SetInternalFieldCount(1); tmpl->SetClassName(conn_str); - AsyncWrap::AddWrapMethods(env, tmpl); + tmpl->Inherit(AsyncWrap::GetConstructorTemplate(env)); env->SetProtoMethod(tmpl, "dispatch", JSBindingsConnection::Dispatch); env->SetProtoMethod(tmpl, "disconnect", JSBindingsConnection::Disconnect); target diff --git a/src/js_stream.cc b/src/js_stream.cc index 9aca2f1d4b5884..99ea7d870b8afb 100644 --- a/src/js_stream.cc +++ b/src/js_stream.cc @@ -202,8 +202,7 @@ void JSStream::Initialize(Local target, FIXED_ONE_BYTE_STRING(env->isolate(), "JSStream"); t->SetClassName(jsStreamString); t->InstanceTemplate()->SetInternalFieldCount(1); - - AsyncWrap::AddWrapMethods(env, t); + t->Inherit(AsyncWrap::GetConstructorTemplate(env)); env->SetProtoMethod(t, "finishWrite", Finish); env->SetProtoMethod(t, "finishShutdown", Finish); diff --git a/src/node_file.cc b/src/node_file.cc index 20cb4771bdfdbe..db78360e6b8e9b 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -2253,7 +2253,7 @@ void Initialize(Local target, // Create FunctionTemplate for FSReqCallback Local fst = env->NewFunctionTemplate(NewFSReqCallback); fst->InstanceTemplate()->SetInternalFieldCount(1); - AsyncWrap::AddWrapMethods(env, fst); + fst->Inherit(AsyncWrap::GetConstructorTemplate(env)); Local wrapString = FIXED_ONE_BYTE_STRING(isolate, "FSReqCallback"); fst->SetClassName(wrapString); @@ -2266,7 +2266,7 @@ void Initialize(Local target, // to do anything in the constructor, so we only store the instance template. Local fh_rw = FunctionTemplate::New(isolate); fh_rw->InstanceTemplate()->SetInternalFieldCount(1); - AsyncWrap::AddWrapMethods(env, fh_rw); + fh_rw->Inherit(AsyncWrap::GetConstructorTemplate(env)); Local fhWrapString = FIXED_ONE_BYTE_STRING(isolate, "FileHandleReqWrap"); fh_rw->SetClassName(fhWrapString); @@ -2275,7 +2275,7 @@ void Initialize(Local target, // Create Function Template for FSReqPromise Local fpt = FunctionTemplate::New(isolate); - AsyncWrap::AddWrapMethods(env, fpt); + fpt->Inherit(AsyncWrap::GetConstructorTemplate(env)); Local promiseString = FIXED_ONE_BYTE_STRING(isolate, "FSReqPromise"); fpt->SetClassName(promiseString); @@ -2285,7 +2285,7 @@ void Initialize(Local target, // Create FunctionTemplate for FileHandle Local fd = env->NewFunctionTemplate(FileHandle::New); - AsyncWrap::AddWrapMethods(env, fd); + fd->Inherit(AsyncWrap::GetConstructorTemplate(env)); env->SetProtoMethod(fd, "close", FileHandle::Close); env->SetProtoMethod(fd, "releaseFD", FileHandle::ReleaseFD); Local fdt = fd->InstanceTemplate(); @@ -2304,7 +2304,7 @@ void Initialize(Local target, Local fdclose = FunctionTemplate::New(isolate); fdclose->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "FileHandleCloseReq")); - AsyncWrap::AddWrapMethods(env, fdclose); + fdclose->Inherit(AsyncWrap::GetConstructorTemplate(env)); Local fdcloset = fdclose->InstanceTemplate(); fdcloset->SetInternalFieldCount(1); env->set_fdclose_constructor_template(fdcloset); diff --git a/src/node_http2.cc b/src/node_http2.cc index 9a0cc4ae191ed0..6a1985ecb741f7 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -2956,14 +2956,14 @@ void Initialize(Local target, Local ping = FunctionTemplate::New(env->isolate()); ping->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "Http2Ping")); - AsyncWrap::AddWrapMethods(env, ping); + ping->Inherit(AsyncWrap::GetConstructorTemplate(env)); Local pingt = ping->InstanceTemplate(); pingt->SetInternalFieldCount(1); env->set_http2ping_constructor_template(pingt); Local setting = FunctionTemplate::New(env->isolate()); setting->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "Http2Setting")); - AsyncWrap::AddWrapMethods(env, setting); + setting->Inherit(AsyncWrap::GetConstructorTemplate(env)); Local settingt = setting->InstanceTemplate(); settingt->SetInternalFieldCount(1); env->set_http2settings_constructor_template(settingt); @@ -2980,7 +2980,7 @@ void Initialize(Local target, env->SetProtoMethod(stream, "respond", Http2Stream::Respond); env->SetProtoMethod(stream, "rstStream", Http2Stream::RstStream); env->SetProtoMethod(stream, "refreshState", Http2Stream::RefreshState); - AsyncWrap::AddWrapMethods(env, stream); + stream->Inherit(AsyncWrap::GetConstructorTemplate(env)); StreamBase::AddMethods(env, stream); Local streamt = stream->InstanceTemplate(); streamt->SetInternalFieldCount(1); @@ -2993,7 +2993,7 @@ void Initialize(Local target, env->NewFunctionTemplate(Http2Session::New); session->SetClassName(http2SessionClassName); session->InstanceTemplate()->SetInternalFieldCount(1); - AsyncWrap::AddWrapMethods(env, session); + session->Inherit(AsyncWrap::GetConstructorTemplate(env)); env->SetProtoMethod(session, "origin", Http2Session::Origin); env->SetProtoMethod(session, "altsvc", Http2Session::AltSvc); env->SetProtoMethod(session, "ping", Http2Session::Ping); diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 3236dd4f9f8b1e..dcaf3d211aee4e 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -763,7 +763,7 @@ void Initialize(Local target, #undef V target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "methods"), methods); - AsyncWrap::AddWrapMethods(env, t); + t->Inherit(AsyncWrap::GetConstructorTemplate(env)); env->SetProtoMethod(t, "close", Parser::Close); env->SetProtoMethod(t, "free", Parser::Free); env->SetProtoMethod(t, "execute", Parser::Execute); diff --git a/src/node_messaging.cc b/src/node_messaging.cc index 6dd66f243e8a20..0a79d6f9d3d36a 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -722,9 +722,7 @@ MaybeLocal GetMessagePortConstructor( Local m = env->NewFunctionTemplate(MessagePort::New); m->SetClassName(env->message_port_constructor_string()); m->InstanceTemplate()->SetInternalFieldCount(1); - - AsyncWrap::AddWrapMethods(env, m); - HandleWrap::AddWrapMethods(env, m); + m->Inherit(HandleWrap::GetConstructorTemplate(env)); env->SetProtoMethod(m, "postMessage", MessagePort::PostMessage); env->SetProtoMethod(m, "start", MessagePort::Start); diff --git a/src/node_stat_watcher.cc b/src/node_stat_watcher.cc index 91333714b28623..ca0927af662200 100644 --- a/src/node_stat_watcher.cc +++ b/src/node_stat_watcher.cc @@ -50,9 +50,7 @@ void StatWatcher::Initialize(Environment* env, Local target) { Local statWatcherString = FIXED_ONE_BYTE_STRING(env->isolate(), "StatWatcher"); t->SetClassName(statWatcherString); - - AsyncWrap::AddWrapMethods(env, t); - HandleWrap::AddWrapMethods(env, t); + t->Inherit(HandleWrap::GetConstructorTemplate(env)); env->SetProtoMethod(t, "start", StatWatcher::Start); diff --git a/src/node_worker.cc b/src/node_worker.cc index aee97095a436b8..63d89a966d14ff 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -482,8 +482,8 @@ void InitWorker(Local target, Local w = env->NewFunctionTemplate(Worker::New); w->InstanceTemplate()->SetInternalFieldCount(1); + w->Inherit(AsyncWrap::GetConstructorTemplate(env)); - AsyncWrap::AddWrapMethods(env, w); env->SetProtoMethod(w, "startThread", Worker::StartThread); env->SetProtoMethod(w, "stopThread", Worker::StopThread); env->SetProtoMethod(w, "ref", Worker::Ref); diff --git a/src/node_wrap.h b/src/node_wrap.h deleted file mode 100644 index 42caca2dc6452d..00000000000000 --- a/src/node_wrap.h +++ /dev/null @@ -1,72 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -#ifndef SRC_NODE_WRAP_H_ -#define SRC_NODE_WRAP_H_ - -#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS - -#include "env.h" -#include "pipe_wrap.h" -#include "tcp_wrap.h" -#include "tty_wrap.h" -#include "uv.h" -#include "v8.h" - -namespace node { - -// TODO(addaleax): Use real inheritance for the JS object templates to avoid -// this unnecessary case switching. -#define WITH_GENERIC_UV_STREAM(env, obj, BODY) \ - do { \ - if (env->tcp_constructor_template().IsEmpty() == false && \ - env->tcp_constructor_template()->HasInstance(obj)) { \ - TCPWrap* const wrap = Unwrap(obj); \ - BODY \ - } else if (env->tty_constructor_template().IsEmpty() == false && \ - env->tty_constructor_template()->HasInstance(obj)) { \ - TTYWrap* const wrap = Unwrap(obj); \ - BODY \ - } else if (env->pipe_constructor_template().IsEmpty() == false && \ - env->pipe_constructor_template()->HasInstance(obj)) { \ - PipeWrap* const wrap = Unwrap(obj); \ - BODY \ - } \ - } while (0) - -inline uv_stream_t* HandleToStream(Environment* env, - v8::Local obj) { - v8::HandleScope scope(env->isolate()); - - WITH_GENERIC_UV_STREAM(env, obj, { - if (wrap == nullptr) - return nullptr; - return reinterpret_cast(wrap->UVHandle()); - }); - - return nullptr; -} - -} // namespace node - -#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS - -#endif // SRC_NODE_WRAP_H_ diff --git a/src/node_zlib.cc b/src/node_zlib.cc index d0cc54c9436582..93dcf0122d2fa7 100644 --- a/src/node_zlib.cc +++ b/src/node_zlib.cc @@ -755,8 +755,8 @@ void Initialize(Local target, Local z = env->NewFunctionTemplate(ZCtx::New); z->InstanceTemplate()->SetInternalFieldCount(1); + z->Inherit(AsyncWrap::GetConstructorTemplate(env)); - AsyncWrap::AddWrapMethods(env, z); env->SetProtoMethod(z, "write", ZCtx::Write); env->SetProtoMethod(z, "writeSync", ZCtx::Write); env->SetProtoMethod(z, "init", ZCtx::Init); diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc index f3317ecc8c2997..eb1e06876c2d86 100644 --- a/src/pipe_wrap.cc +++ b/src/pipe_wrap.cc @@ -28,7 +28,6 @@ #include "node.h" #include "node_buffer.h" #include "node_internals.h" -#include "node_wrap.h" #include "connect_wrap.h" #include "stream_base-inl.h" #include "stream_wrap.h" @@ -78,9 +77,7 @@ void PipeWrap::Initialize(Local target, t->SetClassName(pipeString); t->InstanceTemplate()->SetInternalFieldCount(1); - AsyncWrap::AddWrapMethods(env, t); - HandleWrap::AddWrapMethods(env, t); - LibuvStreamWrap::AddMethods(env, t); + t->Inherit(LibuvStreamWrap::GetConstructorTemplate(env)); env->SetProtoMethod(t, "bind", Bind); env->SetProtoMethod(t, "listen", Listen); @@ -98,7 +95,7 @@ void PipeWrap::Initialize(Local target, // Create FunctionTemplate for PipeConnectWrap. auto cwt = BaseObject::MakeLazilyInitializedJSTemplate(env); - AsyncWrap::AddWrapMethods(env, cwt); + cwt->Inherit(AsyncWrap::GetConstructorTemplate(env)); Local wrapString = FIXED_ONE_BYTE_STRING(env->isolate(), "PipeConnectWrap"); cwt->SetClassName(wrapString); diff --git a/src/process_wrap.cc b/src/process_wrap.cc index 1daa437b296b39..177037588bec05 100644 --- a/src/process_wrap.cc +++ b/src/process_wrap.cc @@ -20,10 +20,9 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. #include "env-inl.h" -#include "handle_wrap.h" #include "node_internals.h" -#include "node_wrap.h" #include "stream_base-inl.h" +#include "stream_wrap.h" #include "util-inl.h" #include @@ -58,8 +57,7 @@ class ProcessWrap : public HandleWrap { FIXED_ONE_BYTE_STRING(env->isolate(), "Process"); constructor->SetClassName(processString); - AsyncWrap::AddWrapMethods(env, constructor); - HandleWrap::AddWrapMethods(env, constructor); + constructor->Inherit(HandleWrap::GetConstructorTemplate(env)); env->SetProtoMethod(constructor, "spawn", Spawn); env->SetProtoMethod(constructor, "kill", Kill); @@ -92,6 +90,18 @@ class ProcessWrap : public HandleWrap { MarkAsUninitialized(); } + static uv_stream_t* StreamForWrap(Environment* env, Local stdio) { + Local handle_key = env->handle_string(); + Local handle = + stdio->Get(env->context(), handle_key).ToLocalChecked().As(); + + Local sw = env->libuv_stream_wrap_ctor_template(); + CHECK(!sw.IsEmpty() && sw->HasInstance(handle)); + uv_stream_t* stream = Unwrap(handle)->stream(); + CHECK_NOT_NULL(stream); + return stream; + } + static void ParseStdioOptions(Environment* env, Local js_options, uv_process_options_t* options) { @@ -115,22 +125,10 @@ class ProcessWrap : public HandleWrap { } else if (type->StrictEquals(env->pipe_string())) { options->stdio[i].flags = static_cast( UV_CREATE_PIPE | UV_READABLE_PIPE | UV_WRITABLE_PIPE); - Local handle_key = env->handle_string(); - Local handle = - stdio->Get(context, handle_key).ToLocalChecked().As(); - CHECK(!handle.IsEmpty()); - options->stdio[i].data.stream = - reinterpret_cast( - Unwrap(handle)->UVHandle()); + options->stdio[i].data.stream = StreamForWrap(env, stdio); } else if (type->StrictEquals(env->wrap_string())) { - Local handle_key = env->handle_string(); - Local handle = - stdio->Get(context, handle_key).ToLocalChecked().As(); - uv_stream_t* stream = HandleToStream(env, handle); - CHECK_NOT_NULL(stream); - options->stdio[i].flags = UV_INHERIT_STREAM; - options->stdio[i].data.stream = stream; + options->stdio[i].data.stream = StreamForWrap(env, stdio); } else { Local fd_key = env->fd_string(); Local fd_value = stdio->Get(context, fd_key).ToLocalChecked(); diff --git a/src/signal_wrap.cc b/src/signal_wrap.cc index b9ec037b453d91..e1792a0267af74 100644 --- a/src/signal_wrap.cc +++ b/src/signal_wrap.cc @@ -50,9 +50,7 @@ class SignalWrap : public HandleWrap { Local signalString = FIXED_ONE_BYTE_STRING(env->isolate(), "Signal"); constructor->SetClassName(signalString); - - AsyncWrap::AddWrapMethods(env, constructor); - HandleWrap::AddWrapMethods(env, constructor); + constructor->Inherit(HandleWrap::GetConstructorTemplate(env)); env->SetProtoMethod(constructor, "start", Start); env->SetProtoMethod(constructor, "stop", Stop); diff --git a/src/stream_pipe.cc b/src/stream_pipe.cc index 90da2043af9c26..0b00f225d8ad7b 100644 --- a/src/stream_pipe.cc +++ b/src/stream_pipe.cc @@ -257,7 +257,7 @@ void InitializeStreamPipe(Local target, FIXED_ONE_BYTE_STRING(env->isolate(), "StreamPipe"); env->SetProtoMethod(pipe, "unpipe", StreamPipe::Unpipe); env->SetProtoMethod(pipe, "start", StreamPipe::Start); - AsyncWrap::AddWrapMethods(env, pipe); + pipe->Inherit(AsyncWrap::GetConstructorTemplate(env)); pipe->SetClassName(stream_pipe_string); pipe->InstanceTemplate()->SetInternalFieldCount(1); target diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index 9738d2a53bddad..51bc4ff30e4a07 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -66,7 +66,7 @@ void LibuvStreamWrap::Initialize(Local target, Local wrapString = FIXED_ONE_BYTE_STRING(env->isolate(), "ShutdownWrap"); sw->SetClassName(wrapString); - AsyncWrap::AddWrapMethods(env, sw); + sw->Inherit(AsyncWrap::GetConstructorTemplate(env)); target->Set(wrapString, sw->GetFunction(env->context()).ToLocalChecked()); env->set_shutdown_wrap_template(sw->InstanceTemplate()); @@ -76,7 +76,7 @@ void LibuvStreamWrap::Initialize(Local target, Local writeWrapString = FIXED_ONE_BYTE_STRING(env->isolate(), "WriteWrap"); ww->SetClassName(writeWrapString); - AsyncWrap::AddWrapMethods(env, ww); + ww->Inherit(AsyncWrap::GetConstructorTemplate(env)); target->Set(writeWrapString, ww->GetFunction(env->context()).ToLocalChecked()); env->set_write_wrap_template(ww->InstanceTemplate()); @@ -96,20 +96,29 @@ LibuvStreamWrap::LibuvStreamWrap(Environment* env, } -void LibuvStreamWrap::AddMethods(Environment* env, - v8::Local target) { - Local get_write_queue_size = - FunctionTemplate::New(env->isolate(), - GetWriteQueueSize, - env->as_external(), - Signature::New(env->isolate(), target)); - target->PrototypeTemplate()->SetAccessorProperty( - env->write_queue_size_string(), - get_write_queue_size, - Local(), - static_cast(ReadOnly | DontDelete)); - env->SetProtoMethod(target, "setBlocking", SetBlocking); - StreamBase::AddMethods(env, target); +Local LibuvStreamWrap::GetConstructorTemplate( + Environment* env) { + Local tmpl = env->libuv_stream_wrap_ctor_template(); + if (tmpl.IsEmpty()) { + tmpl = env->NewFunctionTemplate(nullptr); + tmpl->SetClassName( + FIXED_ONE_BYTE_STRING(env->isolate(), "LibuvStreamWrap")); + tmpl->Inherit(HandleWrap::GetConstructorTemplate(env)); + Local get_write_queue_size = + FunctionTemplate::New(env->isolate(), + GetWriteQueueSize, + env->as_external(), + Signature::New(env->isolate(), tmpl)); + tmpl->PrototypeTemplate()->SetAccessorProperty( + env->write_queue_size_string(), + get_write_queue_size, + Local(), + static_cast(ReadOnly | DontDelete)); + env->SetProtoMethod(tmpl, "setBlocking", SetBlocking); + StreamBase::AddMethods(env, tmpl); + env->set_libuv_stream_wrap_ctor_template(tmpl); + } + return tmpl; } @@ -170,21 +179,25 @@ void LibuvStreamWrap::OnUvAlloc(size_t suggested_size, uv_buf_t* buf) { -template +template static Local AcceptHandle(Environment* env, LibuvStreamWrap* parent) { + static_assert(std::is_base_of::value || + std::is_base_of::value, + "Can only accept stream handles"); + EscapableHandleScope scope(env->isolate()); Local wrap_obj; - UVType* handle; wrap_obj = WrapType::Instantiate(env, parent, WrapType::SOCKET); if (wrap_obj.IsEmpty()) return Local(); - WrapType* wrap; - ASSIGN_OR_RETURN_UNWRAP(&wrap, wrap_obj, Local()); - handle = wrap->UVHandle(); + HandleWrap* wrap = Unwrap(wrap_obj); + CHECK_NOT_NULL(wrap); + uv_stream_t* stream = reinterpret_cast(wrap->GetHandle()); + CHECK_NOT_NULL(stream); - if (uv_accept(parent->stream(), reinterpret_cast(handle))) + if (uv_accept(parent->stream(), stream)) ABORT(); return scope.Escape(wrap_obj); @@ -209,11 +222,11 @@ void LibuvStreamWrap::OnUvRead(ssize_t nread, const uv_buf_t* buf) { Local pending_obj; if (type == UV_TCP) { - pending_obj = AcceptHandle(env(), this); + pending_obj = AcceptHandle(env(), this); } else if (type == UV_NAMED_PIPE) { - pending_obj = AcceptHandle(env(), this); + pending_obj = AcceptHandle(env(), this); } else if (type == UV_UDP) { - pending_obj = AcceptHandle(env(), this); + pending_obj = AcceptHandle(env(), this); } else { CHECK_EQ(type, UV_UNKNOWN_HANDLE); } diff --git a/src/stream_wrap.h b/src/stream_wrap.h index 487a40b7ffc7f9..5eb5ae38e02339 100644 --- a/src/stream_wrap.h +++ b/src/stream_wrap.h @@ -84,8 +84,8 @@ class LibuvStreamWrap : public HandleWrap, public StreamBase { AsyncWrap* GetAsyncWrap() override; - static void AddMethods(Environment* env, - v8::Local target); + static v8::Local GetConstructorTemplate( + Environment* env); protected: inline void set_fd(int fd) { diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index 554b0216fa78e4..ff30be33635a22 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -88,9 +88,7 @@ void TCPWrap::Initialize(Local target, t->InstanceTemplate()->Set(env->onread_string(), Null(env->isolate())); t->InstanceTemplate()->Set(env->onconnection_string(), Null(env->isolate())); - AsyncWrap::AddWrapMethods(env, t, AsyncWrap::kFlagHasReset); - HandleWrap::AddWrapMethods(env, t); - LibuvStreamWrap::AddMethods(env, t); + t->Inherit(LibuvStreamWrap::GetConstructorTemplate(env)); env->SetProtoMethod(t, "open", Open); env->SetProtoMethod(t, "bind", Bind); @@ -115,7 +113,7 @@ void TCPWrap::Initialize(Local target, // Create FunctionTemplate for TCPConnectWrap. Local cwt = BaseObject::MakeLazilyInitializedJSTemplate(env); - AsyncWrap::AddWrapMethods(env, cwt); + cwt->Inherit(AsyncWrap::GetConstructorTemplate(env)); Local wrapString = FIXED_ONE_BYTE_STRING(env->isolate(), "TCPConnectWrap"); cwt->SetClassName(wrapString); diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index 27bedd08cec974..3462dac8ae0d64 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -891,7 +891,7 @@ void TLSWrap::Initialize(Local target, Local(), static_cast(ReadOnly | DontDelete)); - AsyncWrap::AddWrapMethods(env, t, AsyncWrap::kFlagHasReset); + t->Inherit(AsyncWrap::GetConstructorTemplate(env)); env->SetProtoMethod(t, "receive", Receive); env->SetProtoMethod(t, "start", Start); env->SetProtoMethod(t, "setVerifyMode", SetVerifyMode); diff --git a/src/tty_wrap.cc b/src/tty_wrap.cc index e7b684a7eebc02..1d0ea90f36b42c 100644 --- a/src/tty_wrap.cc +++ b/src/tty_wrap.cc @@ -24,7 +24,6 @@ #include "env-inl.h" #include "handle_wrap.h" #include "node_buffer.h" -#include "node_wrap.h" #include "stream_base-inl.h" #include "stream_wrap.h" #include "util-inl.h" @@ -52,10 +51,7 @@ void TTYWrap::Initialize(Local target, Local t = env->NewFunctionTemplate(New); t->SetClassName(ttyString); t->InstanceTemplate()->SetInternalFieldCount(1); - - AsyncWrap::AddWrapMethods(env, t); - HandleWrap::AddWrapMethods(env, t); - LibuvStreamWrap::AddMethods(env, t); + t->Inherit(LibuvStreamWrap::GetConstructorTemplate(env)); env->SetProtoMethodNoSideEffect(t, "getWindowSize", TTYWrap::GetWindowSize); env->SetProtoMethod(t, "setRawMode", SetRawMode); diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index 510926abde2584..77139c2f935bf3 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -135,8 +135,7 @@ void UDPWrap::Initialize(Local target, env->SetProtoMethod(t, "setTTL", SetTTL); env->SetProtoMethod(t, "bufferSize", BufferSize); - AsyncWrap::AddWrapMethods(env, t); - HandleWrap::AddWrapMethods(env, t); + t->Inherit(HandleWrap::GetConstructorTemplate(env)); target->Set(udpString, t->GetFunction(env->context()).ToLocalChecked()); env->set_udp_constructor_function( @@ -145,7 +144,7 @@ void UDPWrap::Initialize(Local target, // Create FunctionTemplate for SendWrap Local swt = BaseObject::MakeLazilyInitializedJSTemplate(env); - AsyncWrap::AddWrapMethods(env, swt); + swt->Inherit(AsyncWrap::GetConstructorTemplate(env)); Local sendWrapString = FIXED_ONE_BYTE_STRING(env->isolate(), "SendWrap"); swt->SetClassName(sendWrapString); diff --git a/test/parallel/test-accessor-properties.js b/test/parallel/test-accessor-properties.js index ce026313d09283..453100d10886ff 100644 --- a/test/parallel/test-accessor-properties.js +++ b/test/parallel/test-accessor-properties.js @@ -32,24 +32,26 @@ const UDP = internalBinding('udp_wrap').UDP; UDP.prototype.fd; }, TypeError); + const StreamWrapProto = Object.getPrototypeOf(TTY.prototype); + // Should not throw for Object.getOwnPropertyDescriptor assert.strictEqual( - typeof Object.getOwnPropertyDescriptor(TTY.prototype, 'bytesRead'), + typeof Object.getOwnPropertyDescriptor(StreamWrapProto, 'bytesRead'), 'object' ); assert.strictEqual( - typeof Object.getOwnPropertyDescriptor(TTY.prototype, 'fd'), + typeof Object.getOwnPropertyDescriptor(StreamWrapProto, 'fd'), 'object' ); assert.strictEqual( - typeof Object.getOwnPropertyDescriptor(TTY.prototype, '_externalStream'), + typeof Object.getOwnPropertyDescriptor(StreamWrapProto, '_externalStream'), 'object' ); assert.strictEqual( - typeof Object.getOwnPropertyDescriptor(UDP.prototype, 'fd'), + typeof Object.getOwnPropertyDescriptor(StreamWrapProto, 'fd'), 'object' ); } From 0b866b668754f545d117f5302048589ab209a358 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 27 Sep 2018 08:26:30 -0400 Subject: [PATCH 2/3] fixup! src: use JS inheritance for `AsyncWrap` --- src/async_wrap.cc | 8 +++++--- src/env.h | 2 +- src/process_wrap.cc | 4 +--- src/stream_wrap.cc | 7 +++++++ src/stream_wrap.h | 2 ++ 5 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/async_wrap.cc b/src/async_wrap.cc index f9e2beaf97338f..eb67433c39f6e5 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -64,8 +64,7 @@ struct AsyncWrapObject : public AsyncWrap { static inline void New(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK(args.IsConstructCall()); - CHECK(env->async_wrap_object_constructor_template()->HasInstance( - args.This())); + CHECK(env->async_wrap_object_ctor_template()->HasInstance(args.This())); CHECK(args[0]->IsUint32()); auto type = static_cast(args[0].As()->Value()); new AsyncWrapObject(env, args.This(), type); @@ -530,6 +529,9 @@ void AsyncWrap::Initialize(Local target, env->set_async_hooks_promise_resolve_function(Local()); env->set_async_hooks_binding(target); + // TODO(addaleax): This block might better work as a + // AsyncWrapObject::Initialize() or AsyncWrapObject::GetConstructorTemplate() + // function. { auto class_name = FIXED_ONE_BYTE_STRING(env->isolate(), "AsyncWrap"); auto function_template = env->NewFunctionTemplate(AsyncWrapObject::New); @@ -540,7 +542,7 @@ void AsyncWrap::Initialize(Local target, auto function = function_template->GetFunction(env->context()).ToLocalChecked(); target->Set(env->context(), class_name, function).FromJust(); - env->set_async_wrap_object_constructor_template(function_template); + env->set_async_wrap_object_ctor_template(function_template); } } diff --git a/src/env.h b/src/env.h index a4a7a2e8a2b631..a210252643c8a4 100644 --- a/src/env.h +++ b/src/env.h @@ -319,7 +319,7 @@ struct PackageConfig { V(async_hooks_destroy_function, v8::Function) \ V(async_hooks_init_function, v8::Function) \ V(async_hooks_promise_resolve_function, v8::Function) \ - V(async_wrap_object_constructor_template, v8::FunctionTemplate) \ + V(async_wrap_object_ctor_template, v8::FunctionTemplate) \ V(async_wrap_ctor_template, v8::FunctionTemplate) \ V(buffer_prototype_object, v8::Object) \ V(context, v8::Context) \ diff --git a/src/process_wrap.cc b/src/process_wrap.cc index 177037588bec05..a46be67cba9e18 100644 --- a/src/process_wrap.cc +++ b/src/process_wrap.cc @@ -95,9 +95,7 @@ class ProcessWrap : public HandleWrap { Local handle = stdio->Get(env->context(), handle_key).ToLocalChecked().As(); - Local sw = env->libuv_stream_wrap_ctor_template(); - CHECK(!sw.IsEmpty() && sw->HasInstance(handle)); - uv_stream_t* stream = Unwrap(handle)->stream(); + uv_stream_t* stream = LibuvStreamWrap::From(env, handle)->stream(); CHECK_NOT_NULL(stream); return stream; } diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index 51bc4ff30e4a07..9ccace435c6796 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -122,6 +122,13 @@ Local LibuvStreamWrap::GetConstructorTemplate( } +LibuvStreamWrap* LibuvStreamWrap::From(Environment* env, Local object) { + Local sw = env->libuv_stream_wrap_ctor_template(); + CHECK(!sw.IsEmpty() && sw->HasInstance(object)); + return Unwrap(object); +} + + int LibuvStreamWrap::GetFD() { #ifdef _WIN32 return fd_; diff --git a/src/stream_wrap.h b/src/stream_wrap.h index 5eb5ae38e02339..98f0ca4ac4fb67 100644 --- a/src/stream_wrap.h +++ b/src/stream_wrap.h @@ -76,6 +76,8 @@ class LibuvStreamWrap : public HandleWrap, public StreamBase { ShutdownWrap* CreateShutdownWrap(v8::Local object) override; WriteWrap* CreateWriteWrap(v8::Local object) override; + static LibuvStreamWrap* From(Environment* env, v8::Local object); + protected: LibuvStreamWrap(Environment* env, v8::Local object, From 70b828b0f262ae399f94a467011494c66bfaf699 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 27 Sep 2018 19:52:52 -0400 Subject: [PATCH 3/3] fixup! src: use JS inheritance for `AsyncWrap` --- src/process_wrap.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/process_wrap.cc b/src/process_wrap.cc index a46be67cba9e18..af6cbfb4e5991b 100644 --- a/src/process_wrap.cc +++ b/src/process_wrap.cc @@ -92,6 +92,7 @@ class ProcessWrap : public HandleWrap { static uv_stream_t* StreamForWrap(Environment* env, Local stdio) { Local handle_key = env->handle_string(); + // This property has always been set by JS land if we are in this code path. Local handle = stdio->Get(env->context(), handle_key).ToLocalChecked().As();