Revert "handle_wrap: IsRefed -> Unrefed, no isAlive check"

Fishrock123 · evanlucas · commit cc2af793d2fd · 2016-05-17T09:32:07.000-05:00
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 <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Trevor Norris <trev.norris@gmail.com> Conflicts: src/handle_wrap.cc test/parallel/test-handle-wrap-isrefed-tty.js test/parallel/test-handle-wrap-isrefed.js
diff --git a/src/handle_wrap.cc b/src/handle_wrap.cc
@@ -34,9 +34,9 @@ void HandleWrap::Unref(const FunctionCallbackInfo<Value>& args){
 }
 
 
-void HandleWrap::Unrefed(const FunctionCallbackInfo<Value>& args){
+void HandleWrap::IsRefed(const FunctionCallbackInfo<Value>& args){
 HandleWrap* wrap = Unwrap<HandleWrap>(args.Holder());
- args.GetReturnValue().Set(!HasRef(wrap));
+ args.GetReturnValue().Set(HasRef(wrap));
 }
 
 
diff --git a/src/handle_wrap.h b/src/handle_wrap.h
@@ -35,7 +35,7 @@ class HandleWrap : public AsyncWrap{
 static void Close(const v8::FunctionCallbackInfo<v8::Value>& args);
 static void Ref(const v8::FunctionCallbackInfo<v8::Value>& args);
 static void Unref(const v8::FunctionCallbackInfo<v8::Value>& args);
- static void Unrefed(const v8::FunctionCallbackInfo<v8::Value>& args);
+ static void IsRefed(const v8::FunctionCallbackInfo<v8::Value>& args);
 
 static inline bool IsAlive(const HandleWrap* wrap){
 return wrap != nullptr && wrap->state_ != kClosed;
diff --git a/src/pipe_wrap.cc b/src/pipe_wrap.cc
@@ -80,7 +80,7 @@ void PipeWrap::Initialize(Local<Object> target,
 env->SetProtoMethod(t, "close", HandleWrap::Close);
 env->SetProtoMethod(t, "unref", HandleWrap::Unref);
 env->SetProtoMethod(t, "ref", HandleWrap::Ref);
- env->SetProtoMethod(t, "unrefed", HandleWrap::Unrefed);
+ env->SetProtoMethod(t, "isRefed", HandleWrap::IsRefed);
 
 StreamWrap::AddMethods(env, t);
 
diff --git a/src/process_wrap.cc b/src/process_wrap.cc
@@ -40,7 +40,7 @@ class ProcessWrap : public HandleWrap{
 
 env->SetProtoMethod(constructor, "ref", HandleWrap::Ref);
 env->SetProtoMethod(constructor, "unref", HandleWrap::Unref);
- env->SetProtoMethod(constructor, "unrefed", HandleWrap::Unrefed);
+ env->SetProtoMethod(constructor, "isRefed", HandleWrap::IsRefed);
 
 target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "Process"),
 constructor->GetFunction());
diff --git a/src/signal_wrap.cc b/src/signal_wrap.cc
@@ -32,7 +32,7 @@ class SignalWrap : public HandleWrap{
 env->SetProtoMethod(constructor, "close", HandleWrap::Close);
 env->SetProtoMethod(constructor, "ref", HandleWrap::Ref);
 env->SetProtoMethod(constructor, "unref", HandleWrap::Unref);
- env->SetProtoMethod(constructor, "unrefed", HandleWrap::Unrefed);
+ env->SetProtoMethod(constructor, "isRefed", HandleWrap::IsRefed);
 env->SetProtoMethod(constructor, "start", Start);
 env->SetProtoMethod(constructor, "stop", Stop);
 
diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc
@@ -87,7 +87,7 @@ void TCPWrap::Initialize(Local<Object> target,
 
 env->SetProtoMethod(t, "ref", HandleWrap::Ref);
 env->SetProtoMethod(t, "unref", HandleWrap::Unref);
- env->SetProtoMethod(t, "unrefed", HandleWrap::Unrefed);
+ env->SetProtoMethod(t, "isRefed", HandleWrap::IsRefed);
 
 StreamWrap::AddMethods(env, t, StreamBase::kFlagHasWritev);
 
diff --git a/src/timer_wrap.cc b/src/timer_wrap.cc
@@ -39,7 +39,7 @@ class TimerWrap : public HandleWrap{
 env->SetProtoMethod(constructor, "close", HandleWrap::Close);
 env->SetProtoMethod(constructor, "ref", HandleWrap::Ref);
 env->SetProtoMethod(constructor, "unref", HandleWrap::Unref);
- env->SetProtoMethod(constructor, "unrefed", HandleWrap::Unrefed);
+ env->SetProtoMethod(constructor, "isRefed", HandleWrap::IsRefed);
 
 env->SetProtoMethod(constructor, "start", Start);
 env->SetProtoMethod(constructor, "stop", Stop);
diff --git a/src/tty_wrap.cc b/src/tty_wrap.cc
@@ -36,7 +36,7 @@ void TTYWrap::Initialize(Local<Object> target,
 
 env->SetProtoMethod(t, "close", HandleWrap::Close);
 env->SetProtoMethod(t, "unref", HandleWrap::Unref);
- env->SetProtoMethod(t, "unrefed", HandleWrap::Unrefed);
+ env->SetProtoMethod(t, "isRefed", HandleWrap::IsRefed);
 
 StreamWrap::AddMethods(env, t, StreamBase::kFlagNoShutdown);
 
diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc
@@ -108,7 +108,7 @@ void UDPWrap::Initialize(Local<Object> target,
 
 env->SetProtoMethod(t, "ref", HandleWrap::Ref);
 env->SetProtoMethod(t, "unref", HandleWrap::Unref);
- env->SetProtoMethod(t, "unrefed", HandleWrap::Unrefed);
+ env->SetProtoMethod(t, "isRefed", HandleWrap::IsRefed);
 
 target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "UDP"), t->GetFunction());
 env->set_udp_constructor_function(t->GetFunction());
diff --git a/test/parallel/test-handle-wrap-isrefed-tty.js b/test/parallel/test-handle-wrap-isrefed-tty.js
@@ -9,19 +9,18 @@ function makeAssert(message){
 strictEqual(actual, expected, message);
 };
 }
-const assert = makeAssert('unrefed() not working on tty_wrap');
+const assert = makeAssert('isRefed() not working on tty_wrap');
 
 if (process.argv[2] === 'child'){
 // Test tty_wrap in piped child to guarentee stdin being a TTY.
 const ReadStream = require('tty').ReadStream;
 const tty = new ReadStream(0);
- assert(Object.getPrototypeOf(tty._handle).hasOwnProperty('unrefed'), true);
- assert(tty._handle.unrefed(), false);
+ assert(Object.getPrototypeOf(tty._handle).hasOwnProperty('isRefed'), true);
+ assert(tty._handle.isRefed(), true);
 tty.unref();
- assert(tty._handle.unrefed(), true);
- tty._handle.close(common.mustCall(() => assert(tty._handle.unrefed(), true)));
- tty._handle.close(common.fail);
- assert(tty._handle.unrefed(), true);
+ assert(tty._handle.isRefed(), false);
+ tty._handle.close(
+ common.mustCall(() => assert(tty._handle.isRefed(), false)));
 return;
 }
 
diff --git a/test/parallel/test-handle-wrap-isrefed.js b/test/parallel/test-handle-wrap-isrefed.js
@@ -12,102 +12,90 @@ function makeAssert(message){
 
 // child_process
{
- const assert = makeAssert('unrefed() not working on process_wrap');
+ const assert = makeAssert('isRefed() not working on process_wrap');
 const spawn = require('child_process').spawn;
 const cmd = common.isWindows ? 'rundll32' : 'ls';
 const cp = spawn(cmd);
- assert(Object.getPrototypeOf(cp._handle).hasOwnProperty('unrefed'), true);
- assert(cp._handle.unrefed(), false);
+ assert(Object.getPrototypeOf(cp._handle).hasOwnProperty('isRefed'), true);
+ assert(cp._handle.isRefed(), true);
 cp.unref();
- assert(cp._handle.unrefed(), true);
+ assert(cp._handle.isRefed(), false);
 cp.ref();
- assert(cp._handle.unrefed(), false);
- cp._handle.close(common.mustCall(() => assert(cp._handle.unrefed(), true)));
- cp._handle.close(common.fail);
- assert(cp._handle.unrefed(), false);
+ assert(cp._handle.isRefed(), true);
+ cp._handle.close(common.mustCall(() => assert(cp._handle.isRefed(), false)));
 }
 
 
 // dgram
{
- const assert = makeAssert('unrefed() not working on udp_wrap');
+ const assert = makeAssert('isRefed() not working on udp_wrap');
 const dgram = require('dgram');
 
 const sock4 = dgram.createSocket('udp4');
- assert(Object.getPrototypeOf(sock4._handle).hasOwnProperty('unrefed'), true);
- assert(sock4._handle.unrefed(), false);
+ assert(Object.getPrototypeOf(sock4._handle).hasOwnProperty('isRefed'), true);
+ assert(sock4._handle.isRefed(), true);
 sock4.unref();
- assert(sock4._handle.unrefed(), true);
+ assert(sock4._handle.isRefed(), false);
 sock4.ref();
- assert(sock4._handle.unrefed(), false);
+ assert(sock4._handle.isRefed(), true);
 sock4._handle.close(
- common.mustCall(() => assert(sock4._handle.unrefed(), true)));
- sock4._handle.close(common.fail);
- assert(sock4._handle.unrefed(), false);
+ common.mustCall(() => assert(sock4._handle.isRefed(), false)));
 
 const sock6 = dgram.createSocket('udp6');
- assert(Object.getPrototypeOf(sock6._handle).hasOwnProperty('unrefed'), true);
- assert(sock6._handle.unrefed(), false);
+ assert(Object.getPrototypeOf(sock6._handle).hasOwnProperty('isRefed'), true);
+ assert(sock6._handle.isRefed(), true);
 sock6.unref();
- assert(sock6._handle.unrefed(), true);
+ assert(sock6._handle.isRefed(), false);
 sock6.ref();
- assert(sock6._handle.unrefed(), false);
+ assert(sock6._handle.isRefed(), true);
 sock6._handle.close(
- common.mustCall(() => assert(sock6._handle.unrefed(), true)));
- sock6._handle.close(common.fail);
- assert(sock6._handle.unrefed(), false);
+ common.mustCall(() => assert(sock6._handle.isRefed(), false)));
 }
 
 
 // pipe
{
- const assert = makeAssert('unrefed() not working on pipe_wrap');
+ const assert = makeAssert('isRefed() not working on pipe_wrap');
 const Pipe = process.binding('pipe_wrap').Pipe;
 const handle = new Pipe();
- assert(Object.getPrototypeOf(handle).hasOwnProperty('unrefed'), true);
- assert(handle.unrefed(), false);
+ assert(Object.getPrototypeOf(handle).hasOwnProperty('isRefed'), true);
+ assert(handle.isRefed(), true);
 handle.unref();
- assert(handle.unrefed(), true);
+ assert(handle.isRefed(), false);
 handle.ref();
- assert(handle.unrefed(), false);
- handle.close(common.mustCall(() => assert(handle.unrefed(), true)));
- handle.close(common.fail);
- assert(handle.unrefed(), false);
+ assert(handle.isRefed(), true);
+ handle.close(common.mustCall(() => assert(handle.isRefed(), false)));
 }
 
 
 // tcp
{
- const assert = makeAssert('unrefed() not working on tcp_wrap');
+ const assert = makeAssert('isRefed() not working on tcp_wrap');
 const net = require('net');
 const server = net.createServer(() =>{}).listen(common.PORT);
- assert(Object.getPrototypeOf(server._handle).hasOwnProperty('unrefed'), true);
- assert(server._handle.unrefed(), false);
+ assert(Object.getPrototypeOf(server._handle).hasOwnProperty('isRefed'), true);
+ assert(server._handle.isRefed(), true);
 assert(server._unref, false);
 server.unref();
- assert(server._handle.unrefed(), true);
+ assert(server._handle.isRefed(), false);
 assert(server._unref, true);
 server.ref();
- assert(server._handle.unrefed(), false);
+ assert(server._handle.isRefed(), true);
 assert(server._unref, false);
 server._handle.close(
- common.mustCall(() => assert(server._handle.unrefed(), true)));
- server._handle.close(common.fail);
- assert(server._handle.unrefed(), false);
+ common.mustCall(() => assert(server._handle.isRefed(), false)));
 }
 
 
 // timers
{
- const assert = makeAssert('unrefed() not working on timer_wrap');
+ const assert = makeAssert('isRefed() not working on timer_wrap');
 const timer = setTimeout(() =>{}, 500);
 timer.unref();
- assert(Object.getPrototypeOf(timer._handle).hasOwnProperty('unrefed'), true);
- assert(timer._handle.unrefed(), true);
+ assert(Object.getPrototypeOf(timer._handle).hasOwnProperty('isRefed'), true);
+ assert(timer._handle.isRefed(), false);
 timer.ref();
- assert(timer._handle.unrefed(), false);
+ assert(timer._handle.isRefed(), true);
 timer._handle.close(
- common.mustCall(() => assert(timer._handle.unrefed(), true)));
- timer._handle.close(common.fail);
- assert(timer._handle.unrefed(), false);
+ common.mustCall(() => assert(timer._handle.isRefed(), false)));
 }

Original file line number	Diff line number	Diff line change
`@@ -34,9 +34,9 @@ void HandleWrap::Unref(const FunctionCallbackInfo<Value>& args){`
`34`	`34`	`}`
`35`	`35`
`36`	`36`
`37`		`-voidHandleWrap::Unrefed(const FunctionCallbackInfo<Value>& args){`
	`37`	`+voidHandleWrap::IsRefed(const FunctionCallbackInfo<Value>& args){`
`38`	`38`	`HandleWrap* wrap = Unwrap<HandleWrap>(args.Holder());`
`39`		`- args.GetReturnValue().Set(!HasRef(wrap));`
	`39`	`+ args.GetReturnValue().Set(HasRef(wrap));`
`40`	`40`	`}`
`41`	`41`
`42`	`42`
-Original file line number
+Diff line change
 staticvoidClose(const v8::FunctionCallbackInfo<v8::Value>& args);
 staticvoidRef(const v8::FunctionCallbackInfo<v8::Value>& args);
 staticvoidUnref(const v8::FunctionCallbackInfo<v8::Value>& args);
 -staticvoidUnrefed(const v8::FunctionCallbackInfo<v8::Value>& args);
 +staticvoidIsRefed(const v8::FunctionCallbackInfo<v8::Value>& args);
 staticinlineboolIsAlive(const HandleWrap* wrap){
 return wrap != nullptr && wrap->state_ != kClosed;
-Original file line number
+Diff line change
 strictEqual(actual,expected,message);
 };
+}
 -constassert=makeAssert('unrefed() not working on tty_wrap');
 +constassert=makeAssert('isRefed() not working on tty_wrap');
 if(process.argv[2]==='child'){
 // Test tty_wrap in piped child to guarentee stdin being a TTY.
 constReadStream=require('tty').ReadStream;
 consttty=newReadStream(0);
 -assert(Object.getPrototypeOf(tty._handle).hasOwnProperty('unrefed'),true);
 -assert(tty._handle.unrefed(),false);
 +assert(Object.getPrototypeOf(tty._handle).hasOwnProperty('isRefed'),true);
 +assert(tty._handle.isRefed(),true);
 tty.unref();
 -assert(tty._handle.unrefed(),true);
 -tty._handle.close(common.mustCall(()=>assert(tty._handle.unrefed(),true)));
 -tty._handle.close(common.fail);
 -assert(tty._handle.unrefed(),true);
 +assert(tty._handle.isRefed(),false);
 +tty._handle.close(
 +common.mustCall(()=>assert(tty._handle.isRefed(),false)));
 return;
+}