Skip to content

Commit 4663393

Browse files
danbevaddaleax
authored andcommitted
src: pull 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. PR-URL: #7547 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1 parent b3127df commit 4663393

9 files changed

+154
-120
lines changed

node.gyp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@
141141
'src/env.cc',
142142
'src/fs_event_wrap.cc',
143143
'src/cares_wrap.cc',
144+
'src/connection_wrap.cc',
144145
'src/handle_wrap.cc',
145146
'src/js_stream.cc',
146147
'src/node.cc',
@@ -177,6 +178,7 @@
177178
'src/async-wrap-inl.h',
178179
'src/base-object.h',
179180
'src/base-object-inl.h',
181+
'src/connection_wrap.h',
180182
'src/debug-agent.h',
181183
'src/env.h',
182184
'src/env-inl.h',

src/base-object-inl.h

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
namespace node {
1414

1515
inline BaseObject::BaseObject(Environment* env, v8::Local<v8::Object> handle)
16-
: handle_(env->isolate(), handle),
16+
: persistent_handle_(env->isolate(), handle),
1717
env_(env) {
1818
CHECK_EQ(false, handle.IsEmpty());
1919
// The zero field holds a pointer to the handle. Immediately set it to
@@ -24,17 +24,17 @@ inline BaseObject::BaseObject(Environment* env, v8::Local<v8::Object> handle)
2424

2525

2626
inline BaseObject::~BaseObject() {
27-
CHECK(handle_.IsEmpty());
27+
CHECK(persistent_handle_.IsEmpty());
2828
}
2929

3030

3131
inline v8::Persistent<v8::Object>& BaseObject::persistent() {
32-
return handle_;
32+
return persistent_handle_;
3333
}
3434

3535

3636
inline v8::Local<v8::Object> BaseObject::object() {
37-
return PersistentToLocal(env_->isolate(), handle_);
37+
return PersistentToLocal(env_->isolate(), persistent_handle_);
3838
}
3939

4040

@@ -58,14 +58,14 @@ inline void BaseObject::MakeWeak(Type* ptr) {
5858
v8::Local<v8::Object> handle = object();
5959
CHECK_GT(handle->InternalFieldCount(), 0);
6060
Wrap(handle, ptr);
61-
handle_.MarkIndependent();
62-
handle_.SetWeak<Type>(ptr, WeakCallback<Type>,
63-
v8::WeakCallbackType::kParameter);
61+
persistent_handle_.MarkIndependent();
62+
persistent_handle_.SetWeak<Type>(ptr, WeakCallback<Type>,
63+
v8::WeakCallbackType::kParameter);
6464
}
6565

6666

6767
inline void BaseObject::ClearWeak() {
68-
handle_.ClearWeak();
68+
persistent_handle_.ClearWeak();
6969
}
7070

7171
} // namespace node

src/base-object.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ class BaseObject {
4444
static inline void WeakCallback(
4545
const v8::WeakCallbackInfo<Type>& data);
4646

47-
v8::Persistent<v8::Object> handle_;
47+
v8::Persistent<v8::Object> persistent_handle_;
4848
Environment* env_;
4949
};
5050

src/connection_wrap.cc

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
#include "connection_wrap.h"
2+
3+
#include "env-inl.h"
4+
#include "env.h"
5+
#include "pipe_wrap.h"
6+
#include "stream_wrap.h"
7+
#include "tcp_wrap.h"
8+
#include "util.h"
9+
#include "util-inl.h"
10+
11+
namespace node {
12+
13+
using v8::Context;
14+
using v8::HandleScope;
15+
using v8::Integer;
16+
using v8::Local;
17+
using v8::Object;
18+
using v8::Value;
19+
20+
21+
template <typename WrapType, typename UVType>
22+
ConnectionWrap<WrapType, UVType>::ConnectionWrap(Environment* env,
23+
Local<Object> object,
24+
ProviderType provider,
25+
AsyncWrap* parent)
26+
: StreamWrap(env,
27+
object,
28+
reinterpret_cast<uv_stream_t*>(&handle_),
29+
provider,
30+
parent) {}
31+
32+
33+
template <typename WrapType, typename UVType>
34+
void ConnectionWrap<WrapType, UVType>::OnConnection(uv_stream_t* handle,
35+
int status) {
36+
WrapType* wrap_data = static_cast<WrapType*>(handle->data);
37+
CHECK_NE(wrap_data, nullptr);
38+
CHECK_EQ(&wrap_data->handle_, reinterpret_cast<UVType*>(handle));
39+
40+
Environment* env = wrap_data->env();
41+
HandleScope handle_scope(env->isolate());
42+
Context::Scope context_scope(env->context());
43+
44+
// We should not be getting this callback if someone has already called
45+
// uv_close() on the handle.
46+
CHECK_EQ(wrap_data->persistent().IsEmpty(), false);
47+
48+
Local<Value> argv[] = {
49+
Integer::New(env->isolate(), status),
50+
Undefined(env->isolate())
51+
};
52+
53+
if (status == 0) {
54+
// Instantiate the client javascript object and handle.
55+
Local<Object> client_obj = WrapType::Instantiate(env, wrap_data);
56+
57+
// Unwrap the client javascript object.
58+
WrapType* wrap;
59+
ASSIGN_OR_RETURN_UNWRAP(&wrap, client_obj);
60+
uv_stream_t* client_handle =
61+
reinterpret_cast<uv_stream_t*>(&wrap->handle_);
62+
// uv_accept can fail if the new connection has already been closed, in
63+
// which case an EAGAIN (resource temporarily unavailable) will be
64+
// returned.
65+
if (uv_accept(handle, client_handle))
66+
return;
67+
68+
// Successful accept. Call the onconnection callback in JavaScript land.
69+
argv[1] = client_obj;
70+
}
71+
wrap_data->MakeCallback(env->onconnection_string(), arraysize(argv), argv);
72+
}
73+
74+
template ConnectionWrap<PipeWrap, uv_pipe_t>::ConnectionWrap(
75+
Environment* env,
76+
Local<Object> object,
77+
ProviderType provider,
78+
AsyncWrap* parent);
79+
80+
template ConnectionWrap<TCPWrap, uv_tcp_t>::ConnectionWrap(
81+
Environment* env,
82+
Local<Object> object,
83+
ProviderType provider,
84+
AsyncWrap* parent);
85+
86+
template void ConnectionWrap<PipeWrap, uv_pipe_t>::OnConnection(
87+
uv_stream_t* handle, int status);
88+
89+
template void ConnectionWrap<TCPWrap, uv_tcp_t>::OnConnection(
90+
uv_stream_t* handle, int status);
91+
92+
93+
} // namespace node

src/connection_wrap.h

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
#ifndef SRC_CONNECTION_WRAP_H_
2+
#define SRC_CONNECTION_WRAP_H_
3+
4+
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
5+
6+
#include "env.h"
7+
#include "stream_wrap.h"
8+
#include "v8.h"
9+
10+
namespace node {
11+
12+
template <typename WrapType, typename UVType>
13+
class ConnectionWrap : public StreamWrap {
14+
public:
15+
UVType* UVHandle() {
16+
return &handle_;
17+
}
18+
19+
static void OnConnection(uv_stream_t* handle, int status);
20+
21+
protected:
22+
ConnectionWrap(Environment* env,
23+
v8::Local<v8::Object> object,
24+
ProviderType provider,
25+
AsyncWrap* parent);
26+
~ConnectionWrap() = default;
27+
28+
UVType handle_;
29+
};
30+
31+
32+
} // namespace node
33+
34+
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
35+
36+
#endif // SRC_CONNECTION_WRAP_H_

src/pipe_wrap.cc

Lines changed: 5 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "pipe_wrap.h"
22

33
#include "async-wrap.h"
4+
#include "connection_wrap.h"
45
#include "env.h"
56
#include "env-inl.h"
67
#include "handle_wrap.h"
@@ -27,7 +28,6 @@ using v8::Integer;
2728
using v8::Local;
2829
using v8::Object;
2930
using v8::String;
30-
using v8::Undefined;
3131
using v8::Value;
3232

3333

@@ -51,11 +51,6 @@ static void NewPipeConnectWrap(const FunctionCallbackInfo<Value>& args) {
5151
}
5252

5353

54-
uv_pipe_t* PipeWrap::UVHandle() {
55-
return &handle_;
56-
}
57-
58-
5954
Local<Object> PipeWrap::Instantiate(Environment* env, AsyncWrap* parent) {
6055
EscapableHandleScope handle_scope(env->isolate());
6156
CHECK_EQ(false, env->pipe_constructor_template().IsEmpty());
@@ -125,11 +120,10 @@ PipeWrap::PipeWrap(Environment* env,
125120
Local<Object> object,
126121
bool ipc,
127122
AsyncWrap* parent)
128-
: StreamWrap(env,
129-
object,
130-
reinterpret_cast<uv_stream_t*>(&handle_),
131-
AsyncWrap::PROVIDER_PIPEWRAP,
132-
parent) {
123+
: ConnectionWrap(env,
124+
object,
125+
AsyncWrap::PROVIDER_PIPEWRAP,
126+
parent) {
133127
int r = uv_pipe_init(env->event_loop(), &handle_, ipc);
134128
CHECK_EQ(r, 0); // How do we proxy this error up to javascript?
135129
// Suggestion: uv_pipe_init() returns void.
@@ -167,44 +161,6 @@ void PipeWrap::Listen(const FunctionCallbackInfo<Value>& args) {
167161
}
168162

169163

170-
// TODO(bnoordhuis) maybe share with TCPWrap?
171-
void PipeWrap::OnConnection(uv_stream_t* handle, int status) {
172-
PipeWrap* pipe_wrap = static_cast<PipeWrap*>(handle->data);
173-
CHECK_EQ(&pipe_wrap->handle_, reinterpret_cast<uv_pipe_t*>(handle));
174-
175-
Environment* env = pipe_wrap->env();
176-
HandleScope handle_scope(env->isolate());
177-
Context::Scope context_scope(env->context());
178-
179-
// We should not be getting this callback if someone as already called
180-
// uv_close() on the handle.
181-
CHECK_EQ(pipe_wrap->persistent().IsEmpty(), false);
182-
183-
Local<Value> argv[] = {
184-
Integer::New(env->isolate(), status),
185-
Undefined(env->isolate())
186-
};
187-
188-
if (status != 0) {
189-
pipe_wrap->MakeCallback(env->onconnection_string(), arraysize(argv), argv);
190-
return;
191-
}
192-
193-
// Instanciate the client javascript object and handle.
194-
Local<Object> client_obj = Instantiate(env, pipe_wrap);
195-
196-
// Unwrap the client javascript object.
197-
PipeWrap* wrap;
198-
ASSIGN_OR_RETURN_UNWRAP(&wrap, client_obj);
199-
uv_stream_t* client_handle = reinterpret_cast<uv_stream_t*>(&wrap->handle_);
200-
if (uv_accept(handle, client_handle))
201-
return;
202-
203-
// Successful accept. Call the onconnection callback in JavaScript land.
204-
argv[1] = client_obj;
205-
pipe_wrap->MakeCallback(env->onconnection_string(), arraysize(argv), argv);
206-
}
207-
208164
// TODO(bnoordhuis) Maybe share this with TCPWrap?
209165
void PipeWrap::AfterConnect(uv_connect_t* req, int status) {
210166
PipeConnectWrap* req_wrap = static_cast<PipeConnectWrap*>(req->data);

src/pipe_wrap.h

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,13 @@
44
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
55

66
#include "async-wrap.h"
7+
#include "connection_wrap.h"
78
#include "env.h"
8-
#include "stream_wrap.h"
99

1010
namespace node {
1111

12-
class PipeWrap : public StreamWrap {
12+
class PipeWrap : public ConnectionWrap<PipeWrap, uv_pipe_t> {
1313
public:
14-
uv_pipe_t* UVHandle();
15-
1614
static v8::Local<v8::Object> Instantiate(Environment* env, AsyncWrap* parent);
1715
static void Initialize(v8::Local<v8::Object> target,
1816
v8::Local<v8::Value> unused,
@@ -37,10 +35,7 @@ class PipeWrap : public StreamWrap {
3735
const v8::FunctionCallbackInfo<v8::Value>& args);
3836
#endif
3937

40-
static void OnConnection(uv_stream_t* handle, int status);
4138
static void AfterConnect(uv_connect_t* req, int status);
42-
43-
uv_pipe_t handle_;
4439
};
4540

4641

0 commit comments

Comments
 (0)