From ad6dee3a74a6a9d1add71396c168e41a2efdac90 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Mon, 4 Jul 2016 19:20:33 +0200 Subject: [PATCH] src: extracting OnConnection from pipe_wrap and tcp_wrap One of the issues in #4641 concerns OnConnection in pipe_wrap and tcp_wrap which are very similar with some minor difference in how they are coded. This commit extracts OnConnection so both these classes can share the same implementation. --- node.gyp | 2 ++ src/connection_wrap.cc | 64 ++++++++++++++++++++++++++++++++++++++++++ src/connection_wrap.h | 22 +++++++++++++++ src/pipe_wrap.cc | 42 ++------------------------- src/pipe_wrap.h | 5 ++-- src/tcp_wrap.cc | 41 ++------------------------- src/tcp_wrap.h | 5 ++-- 7 files changed, 98 insertions(+), 83 deletions(-) create mode 100644 src/connection_wrap.cc create mode 100644 src/connection_wrap.h diff --git a/node.gyp b/node.gyp index a1a5284292a7aa..7ef55cfdaaf1f7 100644 --- a/node.gyp +++ b/node.gyp @@ -141,6 +141,7 @@ 'src/env.cc', 'src/fs_event_wrap.cc', 'src/cares_wrap.cc', + 'src/connection_wrap.cc', 'src/handle_wrap.cc', 'src/js_stream.cc', 'src/node.cc', @@ -177,6 +178,7 @@ 'src/async-wrap-inl.h', 'src/base-object.h', 'src/base-object-inl.h', + 'src/connection_wrap.h', 'src/debug-agent.h', 'src/env.h', 'src/env-inl.h', diff --git a/src/connection_wrap.cc b/src/connection_wrap.cc new file mode 100644 index 00000000000000..88854fa5d51fcf --- /dev/null +++ b/src/connection_wrap.cc @@ -0,0 +1,64 @@ +#include "connection_wrap.h" + +#include "env-inl.h" +#include "env.h" +#include "pipe_wrap.h" +#include "tcp_wrap.h" +#include "util.h" +#include "util-inl.h" + +namespace node { + +using v8::Context; +using v8::HandleScope; +using v8::Integer; +using v8::Local; +using v8::Object; +using v8::Value; + + +template +void ConnectionWrap::OnConnection(uv_stream_t* handle, int status) { + WrapType* wrap_data = static_cast(handle->data); + CHECK_EQ(&wrap_data->handle_, reinterpret_cast(handle)); + + Environment* env = wrap_data->env(); + HandleScope handle_scope(env->isolate()); + Context::Scope context_scope(env->context()); + + // We should not be getting this callback if someone as already called + // uv_close() on the handle. + CHECK_EQ(wrap_data->persistent().IsEmpty(), false); + + Local argv[] = { + Integer::New(env->isolate(), status), + Undefined(env->isolate()) + }; + + if (status == 0) { + // Instanciate the client javascript object and handle. + Local client_obj = WrapType::Instantiate(env, wrap_data); + + // Unwrap the client javascript object. + WrapType* wrap; + ASSIGN_OR_RETURN_UNWRAP(&wrap, client_obj); + uv_stream_t* client_handle = reinterpret_cast(&wrap->handle_); + if (uv_accept(handle, client_handle)) + return; + + // Successful accept. Call the onconnection callback in JavaScript land. + argv[1] = client_obj; + } + wrap_data->MakeCallback(env->onconnection_string(), arraysize(argv), argv); +} + +template void ConnectionWrap::OnConnection( + uv_stream_t* handle, + int status); + +template void ConnectionWrap::OnConnection( + uv_stream_t* handle, + int status); + + +} // namespace node diff --git a/src/connection_wrap.h b/src/connection_wrap.h new file mode 100644 index 00000000000000..06b2dfcfcf8437 --- /dev/null +++ b/src/connection_wrap.h @@ -0,0 +1,22 @@ +#ifndef SRC_CONNECTION_WRAP_H_ +#define SRC_CONNECTION_WRAP_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#include "env.h" +#include "v8.h" + +namespace node { + +class ConnectionWrap { + protected: + template + static void OnConnection(uv_stream_t* handle, int status); +}; + + +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#endif // SRC_CONNECTION_WRAP_H_ diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc index 286ea30a87cf31..b202b621dcb16e 100644 --- a/src/pipe_wrap.cc +++ b/src/pipe_wrap.cc @@ -10,6 +10,7 @@ #include "req-wrap.h" #include "req-wrap-inl.h" #include "stream_wrap.h" +#include "connection_wrap.h" #include "util-inl.h" #include "util.h" @@ -27,7 +28,6 @@ using v8::Integer; using v8::Local; using v8::Object; using v8::String; -using v8::Undefined; using v8::Value; @@ -162,49 +162,11 @@ void PipeWrap::Listen(const FunctionCallbackInfo& args) { int backlog = args[0]->Int32Value(); int err = uv_listen(reinterpret_cast(&wrap->handle_), backlog, - OnConnection); + ConnectionWrap::OnConnection); args.GetReturnValue().Set(err); } -// TODO(bnoordhuis) maybe share with TCPWrap? -void PipeWrap::OnConnection(uv_stream_t* handle, int status) { - PipeWrap* pipe_wrap = static_cast(handle->data); - CHECK_EQ(&pipe_wrap->handle_, reinterpret_cast(handle)); - - Environment* env = pipe_wrap->env(); - HandleScope handle_scope(env->isolate()); - Context::Scope context_scope(env->context()); - - // We should not be getting this callback if someone as already called - // uv_close() on the handle. - CHECK_EQ(pipe_wrap->persistent().IsEmpty(), false); - - Local argv[] = { - Integer::New(env->isolate(), status), - Undefined(env->isolate()) - }; - - if (status != 0) { - pipe_wrap->MakeCallback(env->onconnection_string(), arraysize(argv), argv); - return; - } - - // Instanciate the client javascript object and handle. - Local client_obj = Instantiate(env, pipe_wrap); - - // Unwrap the client javascript object. - PipeWrap* wrap; - ASSIGN_OR_RETURN_UNWRAP(&wrap, client_obj); - uv_stream_t* client_handle = reinterpret_cast(&wrap->handle_); - if (uv_accept(handle, client_handle)) - return; - - // Successful accept. Call the onconnection callback in JavaScript land. - argv[1] = client_obj; - pipe_wrap->MakeCallback(env->onconnection_string(), arraysize(argv), argv); -} - // TODO(bnoordhuis) Maybe share this with TCPWrap? void PipeWrap::AfterConnect(uv_connect_t* req, int status) { PipeConnectWrap* req_wrap = static_cast(req->data); diff --git a/src/pipe_wrap.h b/src/pipe_wrap.h index f4784ac13a089f..b674e5d5d93ac9 100644 --- a/src/pipe_wrap.h +++ b/src/pipe_wrap.h @@ -6,10 +6,11 @@ #include "async-wrap.h" #include "env.h" #include "stream_wrap.h" +#include "connection_wrap.h" namespace node { -class PipeWrap : public StreamWrap { +class PipeWrap : public StreamWrap, ConnectionWrap { public: uv_pipe_t* UVHandle(); @@ -21,6 +22,7 @@ class PipeWrap : public StreamWrap { size_t self_size() const override { return sizeof(*this); } private: + friend class ConnectionWrap; // So handle_ can be accessed PipeWrap(Environment* env, v8::Local object, bool ipc, @@ -37,7 +39,6 @@ class PipeWrap : public StreamWrap { const v8::FunctionCallbackInfo& args); #endif - static void OnConnection(uv_stream_t* handle, int status); static void AfterConnect(uv_connect_t* req, int status); uv_pipe_t handle_; diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index 6904b27efd5e6e..cbd13d6b0e571f 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -8,6 +8,7 @@ #include "req-wrap.h" #include "req-wrap-inl.h" #include "stream_wrap.h" +#include "connection_wrap.h" #include "util.h" #include "util-inl.h" @@ -28,7 +29,6 @@ using v8::Integer; using v8::Local; using v8::Object; using v8::String; -using v8::Undefined; using v8::Value; @@ -253,48 +253,11 @@ void TCPWrap::Listen(const FunctionCallbackInfo& args) { int backlog = args[0]->Int32Value(); int err = uv_listen(reinterpret_cast(&wrap->handle_), backlog, - OnConnection); + ConnectionWrap::OnConnection); args.GetReturnValue().Set(err); } -void TCPWrap::OnConnection(uv_stream_t* handle, int status) { - TCPWrap* tcp_wrap = static_cast(handle->data); - CHECK_EQ(&tcp_wrap->handle_, reinterpret_cast(handle)); - Environment* env = tcp_wrap->env(); - - HandleScope handle_scope(env->isolate()); - Context::Scope context_scope(env->context()); - - // We should not be getting this callback if someone as already called - // uv_close() on the handle. - CHECK_EQ(tcp_wrap->persistent().IsEmpty(), false); - - Local argv[2] = { - Integer::New(env->isolate(), status), - Undefined(env->isolate()) - }; - - if (status == 0) { - // Instantiate the client javascript object and handle. - Local client_obj = - Instantiate(env, static_cast(tcp_wrap)); - - // Unwrap the client javascript object. - TCPWrap* wrap = Unwrap(client_obj); - CHECK_NE(wrap, nullptr); - uv_stream_t* client_handle = reinterpret_cast(&wrap->handle_); - if (uv_accept(handle, client_handle)) - return; - - // Successful accept. Call the onconnection callback in JavaScript land. - argv[1] = client_obj; - } - - tcp_wrap->MakeCallback(env->onconnection_string(), arraysize(argv), argv); -} - - void TCPWrap::AfterConnect(uv_connect_t* req, int status) { TCPConnectWrap* req_wrap = static_cast(req->data); TCPWrap* wrap = static_cast(req->handle->data); diff --git a/src/tcp_wrap.h b/src/tcp_wrap.h index af2d08d1ae80f4..137e337e0ba1d7 100644 --- a/src/tcp_wrap.h +++ b/src/tcp_wrap.h @@ -6,10 +6,11 @@ #include "async-wrap.h" #include "env.h" #include "stream_wrap.h" +#include "connection_wrap.h" namespace node { -class TCPWrap : public StreamWrap { +class TCPWrap : public StreamWrap, ConnectionWrap { public: static v8::Local Instantiate(Environment* env, AsyncWrap* parent); static void Initialize(v8::Local target, @@ -21,6 +22,7 @@ class TCPWrap : public StreamWrap { size_t self_size() const override { return sizeof(*this); } private: + friend class ConnectionWrap; // So handle_ can be accessed typedef uv_tcp_t HandleType; template & args); #endif - static void OnConnection(uv_stream_t* handle, int status); static void AfterConnect(uv_connect_t* req, int status); uv_tcp_t handle_;