Skip to content

Commit cc2af79

Browse files
Fishrock123evanlucas
Fishrock123
authored and
evanlucas
committed
Revert "handle_wrap: IsRefed -> Unrefed, no isAlive check"
This reverts commit 9bb5a5e. This API is not suitable because it depended on being able to potentially access the handle's flag after the handle was already cleaned up. Since this is not actually possible (obviously, oops) this newer API no longer makes much sense, and the older API is more suitable. API comparison: IsRefed -> Has a strong reference AND is alive. (Deterministic) Unrefed -> Has a weak reference OR is dead. (Less deterministic) Refs: #6395 Refs: #6204 Refs: #6401 Refs: #6382 Fixes: #6381 PR-URL: #6546 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Trevor Norris <[email protected]> Conflicts: src/handle_wrap.cc test/parallel/test-handle-wrap-isrefed-tty.js test/parallel/test-handle-wrap-isrefed.js
1 parent 8a7e68f commit cc2af79

11 files changed

+50
-63
lines changed

‎src/handle_wrap.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@ void HandleWrap::Unref(const FunctionCallbackInfo<Value>& args) {
3434
}
3535

3636

37-
void HandleWrap::Unrefed(const FunctionCallbackInfo<Value>& args) {
37+
void HandleWrap::IsRefed(const FunctionCallbackInfo<Value>& args) {
3838
HandleWrap* wrap = Unwrap<HandleWrap>(args.Holder());
39-
args.GetReturnValue().Set(!HasRef(wrap));
39+
args.GetReturnValue().Set(HasRef(wrap));
4040
}
4141

4242

‎src/handle_wrap.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class HandleWrap : public AsyncWrap {
3535
static void Close(const v8::FunctionCallbackInfo<v8::Value>& args);
3636
static void Ref(const v8::FunctionCallbackInfo<v8::Value>& args);
3737
static void Unref(const v8::FunctionCallbackInfo<v8::Value>& args);
38-
static void Unrefed(const v8::FunctionCallbackInfo<v8::Value>& args);
38+
static void IsRefed(const v8::FunctionCallbackInfo<v8::Value>& args);
3939

4040
static inline bool IsAlive(const HandleWrap* wrap) {
4141
return wrap != nullptr && wrap->state_ != kClosed;

‎src/pipe_wrap.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ void PipeWrap::Initialize(Local<Object> target,
8080
env->SetProtoMethod(t, "close", HandleWrap::Close);
8181
env->SetProtoMethod(t, "unref", HandleWrap::Unref);
8282
env->SetProtoMethod(t, "ref", HandleWrap::Ref);
83-
env->SetProtoMethod(t, "unrefed", HandleWrap::Unrefed);
83+
env->SetProtoMethod(t, "isRefed", HandleWrap::IsRefed);
8484

8585
StreamWrap::AddMethods(env, t);
8686

‎src/process_wrap.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class ProcessWrap : public HandleWrap {
4040

4141
env->SetProtoMethod(constructor, "ref", HandleWrap::Ref);
4242
env->SetProtoMethod(constructor, "unref", HandleWrap::Unref);
43-
env->SetProtoMethod(constructor, "unrefed", HandleWrap::Unrefed);
43+
env->SetProtoMethod(constructor, "isRefed", HandleWrap::IsRefed);
4444

4545
target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "Process"),
4646
constructor->GetFunction());

‎src/signal_wrap.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class SignalWrap : public HandleWrap {
3232
env->SetProtoMethod(constructor, "close", HandleWrap::Close);
3333
env->SetProtoMethod(constructor, "ref", HandleWrap::Ref);
3434
env->SetProtoMethod(constructor, "unref", HandleWrap::Unref);
35-
env->SetProtoMethod(constructor, "unrefed", HandleWrap::Unrefed);
35+
env->SetProtoMethod(constructor, "isRefed", HandleWrap::IsRefed);
3636
env->SetProtoMethod(constructor, "start", Start);
3737
env->SetProtoMethod(constructor, "stop", Stop);
3838

‎src/tcp_wrap.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ void TCPWrap::Initialize(Local<Object> target,
8787

8888
env->SetProtoMethod(t, "ref", HandleWrap::Ref);
8989
env->SetProtoMethod(t, "unref", HandleWrap::Unref);
90-
env->SetProtoMethod(t, "unrefed", HandleWrap::Unrefed);
90+
env->SetProtoMethod(t, "isRefed", HandleWrap::IsRefed);
9191

9292
StreamWrap::AddMethods(env, t, StreamBase::kFlagHasWritev);
9393

‎src/timer_wrap.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class TimerWrap : public HandleWrap {
3939
env->SetProtoMethod(constructor, "close", HandleWrap::Close);
4040
env->SetProtoMethod(constructor, "ref", HandleWrap::Ref);
4141
env->SetProtoMethod(constructor, "unref", HandleWrap::Unref);
42-
env->SetProtoMethod(constructor, "unrefed", HandleWrap::Unrefed);
42+
env->SetProtoMethod(constructor, "isRefed", HandleWrap::IsRefed);
4343

4444
env->SetProtoMethod(constructor, "start", Start);
4545
env->SetProtoMethod(constructor, "stop", Stop);

‎src/tty_wrap.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ void TTYWrap::Initialize(Local<Object> target,
3636

3737
env->SetProtoMethod(t, "close", HandleWrap::Close);
3838
env->SetProtoMethod(t, "unref", HandleWrap::Unref);
39-
env->SetProtoMethod(t, "unrefed", HandleWrap::Unrefed);
39+
env->SetProtoMethod(t, "isRefed", HandleWrap::IsRefed);
4040

4141
StreamWrap::AddMethods(env, t, StreamBase::kFlagNoShutdown);
4242

‎src/udp_wrap.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ void UDPWrap::Initialize(Local<Object> target,
108108

109109
env->SetProtoMethod(t, "ref", HandleWrap::Ref);
110110
env->SetProtoMethod(t, "unref", HandleWrap::Unref);
111-
env->SetProtoMethod(t, "unrefed", HandleWrap::Unrefed);
111+
env->SetProtoMethod(t, "isRefed", HandleWrap::IsRefed);
112112

113113
target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "UDP"), t->GetFunction());
114114
env->set_udp_constructor_function(t->GetFunction());

‎test/parallel/test-handle-wrap-isrefed-tty.js

+6-7
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,18 @@ function makeAssert(message) {
99
strictEqual(actual, expected, message);
1010
};
1111
}
12-
const assert = makeAssert('unrefed() not working on tty_wrap');
12+
const assert = makeAssert('isRefed() not working on tty_wrap');
1313

1414
if (process.argv[2] === 'child') {
1515
// Test tty_wrap in piped child to guarentee stdin being a TTY.
1616
const ReadStream = require('tty').ReadStream;
1717
const tty = new ReadStream(0);
18-
assert(Object.getPrototypeOf(tty._handle).hasOwnProperty('unrefed'), true);
19-
assert(tty._handle.unrefed(), false);
18+
assert(Object.getPrototypeOf(tty._handle).hasOwnProperty('isRefed'), true);
19+
assert(tty._handle.isRefed(), true);
2020
tty.unref();
21-
assert(tty._handle.unrefed(), true);
22-
tty._handle.close(common.mustCall(() => assert(tty._handle.unrefed(), true)));
23-
tty._handle.close(common.fail);
24-
assert(tty._handle.unrefed(), true);
21+
assert(tty._handle.isRefed(), false);
22+
tty._handle.close(
23+
common.mustCall(() => assert(tty._handle.isRefed(), false)));
2524
return;
2625
}
2726

‎test/parallel/test-handle-wrap-isrefed.js

+34-46
Original file line numberDiff line numberDiff line change
@@ -12,102 +12,90 @@ function makeAssert(message) {
1212

1313
// child_process
1414
{
15-
const assert = makeAssert('unrefed() not working on process_wrap');
15+
const assert = makeAssert('isRefed() not working on process_wrap');
1616
const spawn = require('child_process').spawn;
1717
const cmd = common.isWindows ? 'rundll32' : 'ls';
1818
const cp = spawn(cmd);
19-
assert(Object.getPrototypeOf(cp._handle).hasOwnProperty('unrefed'), true);
20-
assert(cp._handle.unrefed(), false);
19+
assert(Object.getPrototypeOf(cp._handle).hasOwnProperty('isRefed'), true);
20+
assert(cp._handle.isRefed(), true);
2121
cp.unref();
22-
assert(cp._handle.unrefed(), true);
22+
assert(cp._handle.isRefed(), false);
2323
cp.ref();
24-
assert(cp._handle.unrefed(), false);
25-
cp._handle.close(common.mustCall(() => assert(cp._handle.unrefed(), true)));
26-
cp._handle.close(common.fail);
27-
assert(cp._handle.unrefed(), false);
24+
assert(cp._handle.isRefed(), true);
25+
cp._handle.close(common.mustCall(() => assert(cp._handle.isRefed(), false)));
2826
}
2927

3028

3129
// dgram
3230
{
33-
const assert = makeAssert('unrefed() not working on udp_wrap');
31+
const assert = makeAssert('isRefed() not working on udp_wrap');
3432
const dgram = require('dgram');
3533

3634
const sock4 = dgram.createSocket('udp4');
37-
assert(Object.getPrototypeOf(sock4._handle).hasOwnProperty('unrefed'), true);
38-
assert(sock4._handle.unrefed(), false);
35+
assert(Object.getPrototypeOf(sock4._handle).hasOwnProperty('isRefed'), true);
36+
assert(sock4._handle.isRefed(), true);
3937
sock4.unref();
40-
assert(sock4._handle.unrefed(), true);
38+
assert(sock4._handle.isRefed(), false);
4139
sock4.ref();
42-
assert(sock4._handle.unrefed(), false);
40+
assert(sock4._handle.isRefed(), true);
4341
sock4._handle.close(
44-
common.mustCall(() => assert(sock4._handle.unrefed(), true)));
45-
sock4._handle.close(common.fail);
46-
assert(sock4._handle.unrefed(), false);
42+
common.mustCall(() => assert(sock4._handle.isRefed(), false)));
4743

4844
const sock6 = dgram.createSocket('udp6');
49-
assert(Object.getPrototypeOf(sock6._handle).hasOwnProperty('unrefed'), true);
50-
assert(sock6._handle.unrefed(), false);
45+
assert(Object.getPrototypeOf(sock6._handle).hasOwnProperty('isRefed'), true);
46+
assert(sock6._handle.isRefed(), true);
5147
sock6.unref();
52-
assert(sock6._handle.unrefed(), true);
48+
assert(sock6._handle.isRefed(), false);
5349
sock6.ref();
54-
assert(sock6._handle.unrefed(), false);
50+
assert(sock6._handle.isRefed(), true);
5551
sock6._handle.close(
56-
common.mustCall(() => assert(sock6._handle.unrefed(), true)));
57-
sock6._handle.close(common.fail);
58-
assert(sock6._handle.unrefed(), false);
52+
common.mustCall(() => assert(sock6._handle.isRefed(), false)));
5953
}
6054

6155

6256
// pipe
6357
{
64-
const assert = makeAssert('unrefed() not working on pipe_wrap');
58+
const assert = makeAssert('isRefed() not working on pipe_wrap');
6559
const Pipe = process.binding('pipe_wrap').Pipe;
6660
const handle = new Pipe();
67-
assert(Object.getPrototypeOf(handle).hasOwnProperty('unrefed'), true);
68-
assert(handle.unrefed(), false);
61+
assert(Object.getPrototypeOf(handle).hasOwnProperty('isRefed'), true);
62+
assert(handle.isRefed(), true);
6963
handle.unref();
70-
assert(handle.unrefed(), true);
64+
assert(handle.isRefed(), false);
7165
handle.ref();
72-
assert(handle.unrefed(), false);
73-
handle.close(common.mustCall(() => assert(handle.unrefed(), true)));
74-
handle.close(common.fail);
75-
assert(handle.unrefed(), false);
66+
assert(handle.isRefed(), true);
67+
handle.close(common.mustCall(() => assert(handle.isRefed(), false)));
7668
}
7769

7870

7971
// tcp
8072
{
81-
const assert = makeAssert('unrefed() not working on tcp_wrap');
73+
const assert = makeAssert('isRefed() not working on tcp_wrap');
8274
const net = require('net');
8375
const server = net.createServer(() => {}).listen(common.PORT);
84-
assert(Object.getPrototypeOf(server._handle).hasOwnProperty('unrefed'), true);
85-
assert(server._handle.unrefed(), false);
76+
assert(Object.getPrototypeOf(server._handle).hasOwnProperty('isRefed'), true);
77+
assert(server._handle.isRefed(), true);
8678
assert(server._unref, false);
8779
server.unref();
88-
assert(server._handle.unrefed(), true);
80+
assert(server._handle.isRefed(), false);
8981
assert(server._unref, true);
9082
server.ref();
91-
assert(server._handle.unrefed(), false);
83+
assert(server._handle.isRefed(), true);
9284
assert(server._unref, false);
9385
server._handle.close(
94-
common.mustCall(() => assert(server._handle.unrefed(), true)));
95-
server._handle.close(common.fail);
96-
assert(server._handle.unrefed(), false);
86+
common.mustCall(() => assert(server._handle.isRefed(), false)));
9787
}
9888

9989

10090
// timers
10191
{
102-
const assert = makeAssert('unrefed() not working on timer_wrap');
92+
const assert = makeAssert('isRefed() not working on timer_wrap');
10393
const timer = setTimeout(() => {}, 500);
10494
timer.unref();
105-
assert(Object.getPrototypeOf(timer._handle).hasOwnProperty('unrefed'), true);
106-
assert(timer._handle.unrefed(), true);
95+
assert(Object.getPrototypeOf(timer._handle).hasOwnProperty('isRefed'), true);
96+
assert(timer._handle.isRefed(), false);
10797
timer.ref();
108-
assert(timer._handle.unrefed(), false);
98+
assert(timer._handle.isRefed(), true);
10999
timer._handle.close(
110-
common.mustCall(() => assert(timer._handle.unrefed(), true)));
111-
timer._handle.close(common.fail);
112-
assert(timer._handle.unrefed(), false);
100+
common.mustCall(() => assert(timer._handle.isRefed(), false)));
113101
}

0 commit comments

Comments
 (0)