Skip to content

Commit ba77b4e

Browse files
committed
Refactor delete peer_close_watching_, try_catch, add kPeerCloseCallbackField
1 parent 554052d commit ba77b4e

File tree

2 files changed

+28
-30
lines changed

2 files changed

+28
-30
lines changed

src/pipe_wrap.cc

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -162,11 +162,6 @@ PipeWrap::PipeWrap(Environment* env,
162162
// Suggestion: uv_pipe_init() returns void.
163163
}
164164

165-
PipeWrap::~PipeWrap() {
166-
peer_close_watching_ = false;
167-
peer_close_cb_.Reset();
168-
}
169-
170165
void PipeWrap::Bind(const FunctionCallbackInfo<Value>& args) {
171166
PipeWrap* wrap;
172167
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.This());
@@ -228,16 +223,20 @@ void PipeWrap::WatchPeerClose(const FunctionCallbackInfo<Value>& args) {
228223
CHECK_GT(args.Length(), 0);
229224
CHECK(args[0]->IsBoolean());
230225
const bool enable = args[0].As<v8::Boolean>()->Value();
226+
227+
Environment* env = wrap->env();
228+
Isolate* isolate = env->isolate();
229+
v8::HandleScope handle_scope(isolate);
230+
v8::Context::Scope context_scope(env->context());
231+
Local<Object> obj = wrap->object();
231232

232233
// UnwatchPeerClose
233234
if (!enable) {
234-
if (!wrap->peer_close_watching_) {
235-
wrap->peer_close_cb_.Reset();
235+
if (obj->GetInternalField(kPeerCloseCallbackField).As<Value>()->IsUndefined()) {
236236
return;
237237
}
238238

239-
wrap->peer_close_watching_ = false;
240-
wrap->peer_close_cb_.Reset();
239+
obj->SetInternalField(kPeerCloseCallbackField, v8::Undefined(isolate));
241240
uv_read_stop(wrap->stream());
242241
return;
243242
}
@@ -246,26 +245,21 @@ void PipeWrap::WatchPeerClose(const FunctionCallbackInfo<Value>& args) {
246245
return;
247246
}
248247

249-
if (wrap->peer_close_watching_) {
248+
if (!obj->GetInternalField(kPeerCloseCallbackField).As<Value>()->IsUndefined()) {
250249
return;
251250
}
252251

253252
CHECK_GT(args.Length(), 1);
254253
CHECK(args[1]->IsFunction());
255254

256-
Environment* env = wrap->env();
257-
Isolate* isolate = env->isolate();
258-
259-
// Store the JS callback securely so it isn't garbage collected.
260-
wrap->peer_close_cb_.Reset(isolate, args[1].As<Function>());
261-
wrap->peer_close_watching_ = true;
255+
// Store the JS callback in an internal field.
256+
obj->SetInternalField(kPeerCloseCallbackField, args[1]);
262257

263258
// Start reading to detect EOF/ECONNRESET from the peer.
264259
// We use our custom allocator and reader, ignoring actual data.
265260
int err = uv_read_start(wrap->stream(), PeerCloseAlloc, PeerCloseRead);
266261
if (err != 0) {
267-
wrap->peer_close_watching_ = false;
268-
wrap->peer_close_cb_.Reset();
262+
obj->SetInternalField(kPeerCloseCallbackField, v8::Undefined(isolate));
269263
}
270264
}
271265

@@ -282,7 +276,7 @@ void PipeWrap::PeerCloseRead(uv_stream_t* stream,
282276
ssize_t nread,
283277
const uv_buf_t* buf) {
284278
PipeWrap* wrap = static_cast<PipeWrap*>(stream->data);
285-
if (wrap == nullptr || !wrap->peer_close_watching_) return;
279+
if (wrap == nullptr) return;
286280

287281
// Ignore actual data reads or EAGAIN (0). We only watch for disconnects.
288282
if (nread > 0 || nread == 0) return;
@@ -291,22 +285,25 @@ void PipeWrap::PeerCloseRead(uv_stream_t* stream,
291285
if (nread != UV_EOF && nread != UV_ECONNRESET) return;
292286

293287
// Peer has closed the connection. Stop reading immediately.
294-
wrap->peer_close_watching_ = false;
295288
uv_read_stop(stream);
296289

297-
if (wrap->peer_close_cb_.IsEmpty()) return;
298290
Environment* env = wrap->env();
299291
Isolate* isolate = env->isolate();
300292

301293
// Set up V8 context and handles to safely execute the JS callback.
302294
v8::HandleScope handle_scope(isolate);
303295
v8::Context::Scope context_scope(env->context());
304-
Local<Function> cb = wrap->peer_close_cb_.Get(isolate);
305-
// Reset before calling to prevent re-entrancy issues
306-
wrap->peer_close_cb_.Reset();
296+
Local<Object> obj = wrap->object();
307297

308-
errors::TryCatchScope try_catch(env);
309-
try_catch.SetVerbose(true);
298+
// Check if callback is set
299+
if (obj->GetInternalField(kPeerCloseCallbackField).As<Value>()->IsUndefined()) {
300+
return;
301+
}
302+
303+
Local<Value> cb_value = obj->GetInternalField(kPeerCloseCallbackField).As<Value>();
304+
Local<Function> cb = cb_value.As<Function>();
305+
// Reset before calling to prevent re-entrancy issues
306+
obj->SetInternalField(kPeerCloseCallbackField, v8::Undefined(isolate));
310307

311308
// MakeCallback properly tracks AsyncHooks context and flushes microtasks.
312309
wrap->MakeCallback(cb, 0, nullptr);

src/pipe_wrap.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@ class PipeWrap : public ConnectionWrap<PipeWrap, uv_pipe_t> {
4040
IPC
4141
};
4242

43+
enum InternalFields {
44+
kPeerCloseCallbackField = LibuvStreamWrap::kInternalFieldCount,
45+
kInternalFieldCount
46+
};
47+
4348
static v8::MaybeLocal<v8::Object> Instantiate(Environment* env,
4449
AsyncWrap* parent,
4550
SocketType type);
@@ -54,7 +59,6 @@ class PipeWrap : public ConnectionWrap<PipeWrap, uv_pipe_t> {
5459
SET_SELF_SIZE(PipeWrap)
5560

5661
private:
57-
~PipeWrap() override;
5862
PipeWrap(Environment* env,
5963
v8::Local<v8::Object> object,
6064
ProviderType provider,
@@ -78,9 +82,6 @@ class PipeWrap : public ConnectionWrap<PipeWrap, uv_pipe_t> {
7882
const v8::FunctionCallbackInfo<v8::Value>& args);
7983
#endif
8084
static void Fchmod(const v8::FunctionCallbackInfo<v8::Value>& args);
81-
82-
bool peer_close_watching_ = false;
83-
v8::Global<v8::Function> peer_close_cb_;
8485
};
8586

8687

0 commit comments

Comments
 (0)