From 3b06d2998447b78145c1fd2cf697822f8e56da08 Mon Sep 17 00:00:00 2001 From: Jon Moss Date: Tue, 18 Dec 2018 15:52:09 -0500 Subject: [PATCH] src: remove templating from StreamBase PR-URL: https://github.com/nodejs/node/pull/25142 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell --- src/js_stream.cc | 6 +- src/node_file.cc | 5 +- src/node_http2.cc | 5 +- src/pipe_wrap.cc | 3 +- src/stream_base-inl.h | 142 ++----------------------- src/stream_base.cc | 87 +++++++++++++++ src/stream_base.h | 26 ++--- src/stream_wrap.cc | 5 +- src/tcp_wrap.cc | 3 +- src/tls_wrap.cc | 5 +- src/tty_wrap.cc | 3 +- test/parallel/test-tls-socket-close.js | 2 +- 12 files changed, 133 insertions(+), 159 deletions(-) diff --git a/src/js_stream.cc b/src/js_stream.cc index ae1aa7cd30e949..e5f4273476423a 100644 --- a/src/js_stream.cc +++ b/src/js_stream.cc @@ -27,6 +27,7 @@ JSStream::JSStream(Environment* env, Local obj) : AsyncWrap(env, obj, AsyncWrap::PROVIDER_JSSTREAM), StreamBase(env) { MakeWeak(); + StreamBase::AttachToObject(obj); } @@ -203,7 +204,8 @@ void JSStream::Initialize(Local target, Local jsStreamString = FIXED_ONE_BYTE_STRING(env->isolate(), "JSStream"); t->SetClassName(jsStreamString); - t->InstanceTemplate()->SetInternalFieldCount(1); + t->InstanceTemplate() + ->SetInternalFieldCount(StreamBase::kStreamBaseField + 1); t->Inherit(AsyncWrap::GetConstructorTemplate(env)); env->SetProtoMethod(t, "finishWrite", Finish); @@ -211,7 +213,7 @@ void JSStream::Initialize(Local target, env->SetProtoMethod(t, "readBuffer", ReadBuffer); env->SetProtoMethod(t, "emitEOF", EmitEOF); - StreamBase::AddMethods(env, t); + StreamBase::AddMethods(env, t); target->Set(env->context(), jsStreamString, t->GetFunction(context).ToLocalChecked()).FromJust(); diff --git a/src/node_file.cc b/src/node_file.cc index dd658e2ecf0829..f4106ce6477678 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -111,6 +111,7 @@ FileHandle::FileHandle(Environment* env, Local obj, int fd) StreamBase(env), fd_(fd) { MakeWeak(); + StreamBase::AttachToObject(GetObject()); } FileHandle* FileHandle::New(Environment* env, int fd, Local obj) { @@ -2227,11 +2228,11 @@ void Initialize(Local target, env->SetProtoMethod(fd, "close", FileHandle::Close); env->SetProtoMethod(fd, "releaseFD", FileHandle::ReleaseFD); Local fdt = fd->InstanceTemplate(); - fdt->SetInternalFieldCount(1); + fdt->SetInternalFieldCount(StreamBase::kStreamBaseField + 1); Local handleString = FIXED_ONE_BYTE_STRING(isolate, "FileHandle"); fd->SetClassName(handleString); - StreamBase::AddMethods(env, fd); + StreamBase::AddMethods(env, fd); target ->Set(context, handleString, fd->GetFunction(env->context()).ToLocalChecked()) diff --git a/src/node_http2.cc b/src/node_http2.cc index fecde4f5227088..f52e3b97910efe 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1868,6 +1868,7 @@ Http2Stream::Http2Stream(Http2Session* session, id_(id), current_headers_category_(category) { MakeWeak(); + StreamBase::AttachToObject(GetObject()); statistics_.start_time = uv_hrtime(); // Limit the number of header pairs @@ -3009,9 +3010,9 @@ void Initialize(Local target, env->SetProtoMethod(stream, "rstStream", Http2Stream::RstStream); env->SetProtoMethod(stream, "refreshState", Http2Stream::RefreshState); stream->Inherit(AsyncWrap::GetConstructorTemplate(env)); - StreamBase::AddMethods(env, stream); + StreamBase::AddMethods(env, stream); Local streamt = stream->InstanceTemplate(); - streamt->SetInternalFieldCount(1); + streamt->SetInternalFieldCount(StreamBase::kStreamBaseField + 1); env->set_http2stream_constructor_template(streamt); target->Set(context, FIXED_ONE_BYTE_STRING(env->isolate(), "Http2Stream"), diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc index 2354533ddf3e43..0103129a66c5ab 100644 --- a/src/pipe_wrap.cc +++ b/src/pipe_wrap.cc @@ -74,7 +74,8 @@ void PipeWrap::Initialize(Local target, Local t = env->NewFunctionTemplate(New); Local pipeString = FIXED_ONE_BYTE_STRING(env->isolate(), "Pipe"); t->SetClassName(pipeString); - t->InstanceTemplate()->SetInternalFieldCount(1); + t->InstanceTemplate() + ->SetInternalFieldCount(StreamBase::kStreamBaseField + 1); t->Inherit(LibuvStreamWrap::GetConstructorTemplate(env)); diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h index 9cff67cd9fecf2..f9a67872f90d77 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -273,144 +273,16 @@ inline WriteWrap* StreamBase::CreateWriteWrap( return new SimpleWriteWrap(this, object); } -template -void StreamBase::AddMethods(Environment* env, Local t) { - HandleScope scope(env->isolate()); - - enum PropertyAttribute attributes = - static_cast( - v8::ReadOnly | v8::DontDelete | v8::DontEnum); - - Local signature = Signature::New(env->isolate(), t); - - Local get_fd_templ = - env->NewFunctionTemplate(GetFD, - signature, - v8::ConstructorBehavior::kThrow, - v8::SideEffectType::kHasNoSideEffect); - - Local get_external_templ = - env->NewFunctionTemplate(GetExternal, - signature, - v8::ConstructorBehavior::kThrow, - v8::SideEffectType::kHasNoSideEffect); - - Local get_bytes_read_templ = - env->NewFunctionTemplate(GetBytesRead, - signature, - v8::ConstructorBehavior::kThrow, - v8::SideEffectType::kHasNoSideEffect); - - Local get_bytes_written_templ = - env->NewFunctionTemplate(GetBytesWritten, - signature, - v8::ConstructorBehavior::kThrow, - v8::SideEffectType::kHasNoSideEffect); - - t->PrototypeTemplate()->SetAccessorProperty(env->fd_string(), - get_fd_templ, - Local(), - attributes); - - t->PrototypeTemplate()->SetAccessorProperty(env->external_stream_string(), - get_external_templ, - Local(), - attributes); - - t->PrototypeTemplate()->SetAccessorProperty(env->bytes_read_string(), - get_bytes_read_templ, - Local(), - attributes); - - t->PrototypeTemplate()->SetAccessorProperty(env->bytes_written_string(), - get_bytes_written_templ, - Local(), - attributes); - - env->SetProtoMethod(t, "readStart", JSMethod); - env->SetProtoMethod(t, "readStop", JSMethod); - env->SetProtoMethod(t, "shutdown", JSMethod); - env->SetProtoMethod(t, "writev", JSMethod); - env->SetProtoMethod(t, - "writeBuffer", - JSMethod); - env->SetProtoMethod(t, - "writeAsciiString", - JSMethod >); - env->SetProtoMethod(t, - "writeUtf8String", - JSMethod >); - env->SetProtoMethod(t, - "writeUcs2String", - JSMethod >); - env->SetProtoMethod(t, - "writeLatin1String", - JSMethod >); +inline void StreamBase::AttachToObject(v8::Local obj) { + obj->SetAlignedPointerInInternalField(kStreamBaseField, this); } +inline StreamBase* StreamBase::FromObject(v8::Local obj) { + if (obj->GetAlignedPointerFromInternalField(0) == nullptr) + return nullptr; -template -void StreamBase::GetFD(const FunctionCallbackInfo& args) { - // Mimic implementation of StreamBase::GetFD() and UDPWrap::GetFD(). - Base* handle; - ASSIGN_OR_RETURN_UNWRAP(&handle, - args.This(), - args.GetReturnValue().Set(UV_EINVAL)); - - StreamBase* wrap = static_cast(handle); - if (!wrap->IsAlive()) - return args.GetReturnValue().Set(UV_EINVAL); - - args.GetReturnValue().Set(wrap->GetFD()); -} - -template -void StreamBase::GetBytesRead(const FunctionCallbackInfo& args) { - Base* handle; - ASSIGN_OR_RETURN_UNWRAP(&handle, - args.This(), - args.GetReturnValue().Set(0)); - - StreamBase* wrap = static_cast(handle); - // uint64_t -> double. 53bits is enough for all real cases. - args.GetReturnValue().Set(static_cast(wrap->bytes_read_)); -} - -template -void StreamBase::GetBytesWritten(const FunctionCallbackInfo& args) { - Base* handle; - ASSIGN_OR_RETURN_UNWRAP(&handle, - args.This(), - args.GetReturnValue().Set(0)); - - StreamBase* wrap = static_cast(handle); - // uint64_t -> double. 53bits is enough for all real cases. - args.GetReturnValue().Set(static_cast(wrap->bytes_written_)); -} - -template -void StreamBase::GetExternal(const FunctionCallbackInfo& args) { - Base* handle; - ASSIGN_OR_RETURN_UNWRAP(&handle, args.This()); - - StreamBase* wrap = static_cast(handle); - Local ext = External::New(args.GetIsolate(), wrap); - args.GetReturnValue().Set(ext); -} - - -template & args)> -void StreamBase::JSMethod(const FunctionCallbackInfo& args) { - Base* handle; - ASSIGN_OR_RETURN_UNWRAP(&handle, args.Holder()); - - StreamBase* wrap = static_cast(handle); - if (!wrap->IsAlive()) - return args.GetReturnValue().Set(UV_EINVAL); - - AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(handle); - args.GetReturnValue().Set((wrap->*Method)(args)); + return static_cast( + obj->GetAlignedPointerFromInternalField(kStreamBaseField)); } diff --git a/src/stream_base.cc b/src/stream_base.cc index 24210e1e260b24..bf7003e12764e7 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -327,6 +327,93 @@ Local StreamBase::GetObject() { return GetAsyncWrap()->object(); } +void StreamBase::AddMethod(Environment* env, + Local signature, + enum PropertyAttribute attributes, + Local t, + JSMethodFunction* stream_method, + Local string) { + Local templ = + env->NewFunctionTemplate(stream_method, + signature, + v8::ConstructorBehavior::kThrow, + v8::SideEffectType::kHasNoSideEffect); + t->PrototypeTemplate()->SetAccessorProperty( + string, templ, Local(), attributes); +} + +void StreamBase::AddMethods(Environment* env, Local t) { + HandleScope scope(env->isolate()); + + enum PropertyAttribute attributes = static_cast( + v8::ReadOnly | v8::DontDelete | v8::DontEnum); + Local sig = Signature::New(env->isolate(), t); + + AddMethod(env, sig, attributes, t, GetFD, env->fd_string()); + AddMethod( + env, sig, attributes, t, GetExternal, env->external_stream_string()); + AddMethod(env, sig, attributes, t, GetBytesRead, env->bytes_read_string()); + AddMethod( + env, sig, attributes, t, GetBytesWritten, env->bytes_written_string()); + env->SetProtoMethod(t, "readStart", JSMethod<&StreamBase::ReadStartJS>); + env->SetProtoMethod(t, "readStop", JSMethod<&StreamBase::ReadStopJS>); + env->SetProtoMethod(t, "shutdown", JSMethod<&StreamBase::Shutdown>); + env->SetProtoMethod(t, "writev", JSMethod<&StreamBase::Writev>); + env->SetProtoMethod(t, "writeBuffer", JSMethod<&StreamBase::WriteBuffer>); + env->SetProtoMethod( + t, "writeAsciiString", JSMethod<&StreamBase::WriteString>); + env->SetProtoMethod( + t, "writeUtf8String", JSMethod<&StreamBase::WriteString>); + env->SetProtoMethod( + t, "writeUcs2String", JSMethod<&StreamBase::WriteString>); + env->SetProtoMethod( + t, "writeLatin1String", JSMethod<&StreamBase::WriteString>); +} + +void StreamBase::GetFD(const FunctionCallbackInfo& args) { + // Mimic implementation of StreamBase::GetFD() and UDPWrap::GetFD(). + StreamBase* wrap = StreamBase::FromObject(args.This().As()); + if (wrap == nullptr) return args.GetReturnValue().Set(UV_EINVAL); + + if (!wrap->IsAlive()) return args.GetReturnValue().Set(UV_EINVAL); + + args.GetReturnValue().Set(wrap->GetFD()); +} + +void StreamBase::GetBytesRead(const FunctionCallbackInfo& args) { + StreamBase* wrap = StreamBase::FromObject(args.This().As()); + if (wrap == nullptr) return args.GetReturnValue().Set(0); + + // uint64_t -> double. 53bits is enough for all real cases. + args.GetReturnValue().Set(static_cast(wrap->bytes_read_)); +} + +void StreamBase::GetBytesWritten(const FunctionCallbackInfo& args) { + StreamBase* wrap = StreamBase::FromObject(args.This().As()); + if (wrap == nullptr) return args.GetReturnValue().Set(0); + + // uint64_t -> double. 53bits is enough for all real cases. + args.GetReturnValue().Set(static_cast(wrap->bytes_written_)); +} + +void StreamBase::GetExternal(const FunctionCallbackInfo& args) { + StreamBase* wrap = StreamBase::FromObject(args.This().As()); + if (wrap == nullptr) return; + + Local ext = External::New(args.GetIsolate(), wrap); + args.GetReturnValue().Set(ext); +} + +template & args)> +void StreamBase::JSMethod(const FunctionCallbackInfo& args) { + StreamBase* wrap = StreamBase::FromObject(args.Holder().As()); + if (wrap == nullptr) return; + + if (!wrap->IsAlive()) return args.GetReturnValue().Set(UV_EINVAL); + + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(wrap->GetAsyncWrap()); + args.GetReturnValue().Set((wrap->*Method)(args)); +} int StreamResource::DoTryWrite(uv_buf_t** bufs, size_t* count) { // No TryWrite by default diff --git a/src/stream_base.h b/src/stream_base.h index bf15dbc91e05f0..c7145a2ab2b236 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -25,6 +25,7 @@ struct StreamWriteResult { size_t bytes; }; +using JSMethodFunction = void(const v8::FunctionCallbackInfo& args); class StreamReq { public: @@ -259,9 +260,9 @@ class StreamResource { class StreamBase : public StreamResource { public: - template - static inline void AddMethods(Environment* env, - v8::Local target); + static constexpr int kStreamBaseField = 1; + static void AddMethods(Environment* env, + v8::Local target); virtual bool IsAlive() = 0; virtual bool IsClosing() = 0; @@ -305,6 +306,8 @@ class StreamBase : public StreamResource { virtual AsyncWrap* GetAsyncWrap() = 0; virtual v8::Local GetObject(); + static StreamBase* FromObject(v8::Local obj); + protected: explicit StreamBase(Environment* env); @@ -317,20 +320,13 @@ class StreamBase : public StreamResource { template int WriteString(const v8::FunctionCallbackInfo& args); - template static void GetFD(const v8::FunctionCallbackInfo& args); - - template static void GetExternal(const v8::FunctionCallbackInfo& args); - - template static void GetBytesRead(const v8::FunctionCallbackInfo& args); - - template static void GetBytesWritten(const v8::FunctionCallbackInfo& args); + void AttachToObject(v8::Local obj); - template & args)> static void JSMethod(const v8::FunctionCallbackInfo& args); @@ -348,6 +344,12 @@ class StreamBase : public StreamResource { EmitToJSStreamListener default_listener_; void SetWriteResult(const StreamWriteResult& res); + static void AddMethod(Environment* env, + v8::Local sig, + enum v8::PropertyAttribute attributes, + v8::Local t, + JSMethodFunction* stream_method, + v8::Local str); friend class WriteWrap; friend class ShutdownWrap; diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index 7e1ba369220153..0cf9f1a114c64d 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -123,6 +123,7 @@ LibuvStreamWrap::LibuvStreamWrap(Environment* env, provider), StreamBase(env), stream_(stream) { + StreamBase::AttachToObject(object); } @@ -134,6 +135,8 @@ Local LibuvStreamWrap::GetConstructorTemplate( tmpl->SetClassName( FIXED_ONE_BYTE_STRING(env->isolate(), "LibuvStreamWrap")); tmpl->Inherit(HandleWrap::GetConstructorTemplate(env)); + tmpl->InstanceTemplate()->SetInternalFieldCount( + StreamBase::kStreamBaseField + 1); Local get_write_queue_size = FunctionTemplate::New(env->isolate(), GetWriteQueueSize, @@ -145,7 +148,7 @@ Local LibuvStreamWrap::GetConstructorTemplate( Local(), static_cast(ReadOnly | DontDelete)); env->SetProtoMethod(tmpl, "setBlocking", SetBlocking); - StreamBase::AddMethods(env, tmpl); + StreamBase::AddMethods(env, tmpl); env->set_libuv_stream_wrap_ctor_template(tmpl); } return tmpl; diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index 43c6ea5958fa22..af9806e4889b4c 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -79,7 +79,8 @@ void TCPWrap::Initialize(Local target, Local t = env->NewFunctionTemplate(New); Local tcpString = FIXED_ONE_BYTE_STRING(env->isolate(), "TCP"); t->SetClassName(tcpString); - t->InstanceTemplate()->SetInternalFieldCount(1); + t->InstanceTemplate() + ->SetInternalFieldCount(StreamBase::kStreamBaseField + 1); // Init properties t->InstanceTemplate()->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "reading"), diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index 4e5acbfae3785a..1a546df843557c 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -57,6 +57,7 @@ TLSWrap::TLSWrap(Environment* env, StreamBase(env), sc_(sc) { MakeWeak(); + StreamBase::AttachToObject(GetObject()); // sc comes from an Unwrap. Make sure it was assigned. CHECK_NOT_NULL(sc); @@ -923,6 +924,8 @@ void TLSWrap::Initialize(Local target, Local tlsWrapString = FIXED_ONE_BYTE_STRING(env->isolate(), "TLSWrap"); t->SetClassName(tlsWrapString); + t->InstanceTemplate() + ->SetInternalFieldCount(StreamBase::kStreamBaseField + 1); Local get_write_queue_size = FunctionTemplate::New(env->isolate(), @@ -943,7 +946,7 @@ void TLSWrap::Initialize(Local target, env->SetProtoMethod(t, "destroySSL", DestroySSL); env->SetProtoMethod(t, "enableCertCb", EnableCertCb); - StreamBase::AddMethods(env, t); + StreamBase::AddMethods(env, t); SSLWrap::AddMethods(env, t); env->SetProtoMethod(t, "getServername", GetServername); diff --git a/src/tty_wrap.cc b/src/tty_wrap.cc index 7b8a7ed227446d..423f53c014d784 100644 --- a/src/tty_wrap.cc +++ b/src/tty_wrap.cc @@ -51,7 +51,8 @@ void TTYWrap::Initialize(Local target, Local t = env->NewFunctionTemplate(New); t->SetClassName(ttyString); - t->InstanceTemplate()->SetInternalFieldCount(1); + t->InstanceTemplate() + ->SetInternalFieldCount(StreamBase::kStreamBaseField + 1); t->Inherit(LibuvStreamWrap::GetConstructorTemplate(env)); env->SetProtoMethodNoSideEffect(t, "getWindowSize", TTYWrap::GetWindowSize); diff --git a/test/parallel/test-tls-socket-close.js b/test/parallel/test-tls-socket-close.js index 4fe58bc7f2dbf2..73130768890550 100644 --- a/test/parallel/test-tls-socket-close.js +++ b/test/parallel/test-tls-socket-close.js @@ -55,7 +55,7 @@ function connectClient(server) { tlsConnection.write('foo', 'utf8', common.mustCall(() => { assert(netSocket); - netSocket.setTimeout(1, common.mustCall(() => { + netSocket.setTimeout(common.platformTimeout(10), common.mustCall(() => { assert(tlsSocket); // This breaks if TLSSocket is already managing the socket: netSocket.destroy();