Skip to content

Commit

Permalink
handle_wrap: IsRefed -> Unrefed, no isAlive check
Browse files Browse the repository at this point in the history
This fixes my perceived usability issues with 7d8882b. Which, at the
time of writing, has not landed in any release except v6 RCs. This
should not be considered a breaking change due to that.

It is useful if you have a handle, even if it has been closed, to be
able to inspect whether that handle was unrefed or not. As such, this
renames the method accordingly. If people need to check a handle's
aliveness, that is a separate API we should consider exposing.

Refs: #5834
PR-URL: #6204
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
  • Loading branch information
Fishrock123 authored and evanlucas committed May 17, 2016
1 parent 316871f commit 2000072
Show file tree
Hide file tree
Showing 11 changed files with 57 additions and 49 deletions.
6 changes: 3 additions & 3 deletions src/handle_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ void HandleWrap::Unref(const FunctionCallbackInfo<Value>& args) {
}


void HandleWrap::IsRefed(const FunctionCallbackInfo<Value>& args) {
void HandleWrap::Unrefed(const FunctionCallbackInfo<Value>& args) {
HandleWrap* wrap = Unwrap<HandleWrap>(args.Holder());

bool refed = IsAlive(wrap) && (wrap->flags_ & kUnref) == 0;
args.GetReturnValue().Set(refed);
bool unrefed = wrap->flags_ & kUnref == 1;
args.GetReturnValue().Set(unrefed);
}


Expand Down
2 changes: 1 addition & 1 deletion src/handle_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 IsRefed(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Unrefed(const v8::FunctionCallbackInfo<v8::Value>& args);

static inline bool IsAlive(const HandleWrap* wrap) {
return wrap != nullptr && wrap->GetHandle() != nullptr;
Expand Down
2 changes: 1 addition & 1 deletion src/pipe_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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, "isRefed", HandleWrap::IsRefed);
env->SetProtoMethod(t, "unrefed", HandleWrap::Unrefed);

StreamWrap::AddMethods(env, t);

Expand Down
2 changes: 1 addition & 1 deletion src/process_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class ProcessWrap : public HandleWrap {

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

target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "Process"),
constructor->GetFunction());
Expand Down
2 changes: 1 addition & 1 deletion src/signal_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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, "isRefed", HandleWrap::IsRefed);
env->SetProtoMethod(constructor, "unrefed", HandleWrap::Unrefed);
env->SetProtoMethod(constructor, "start", Start);
env->SetProtoMethod(constructor, "stop", Stop);

Expand Down
2 changes: 1 addition & 1 deletion src/tcp_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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, "isRefed", HandleWrap::IsRefed);
env->SetProtoMethod(t, "unrefed", HandleWrap::Unrefed);

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

Expand Down
2 changes: 1 addition & 1 deletion src/timer_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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, "isRefed", HandleWrap::IsRefed);
env->SetProtoMethod(constructor, "unrefed", HandleWrap::Unrefed);

env->SetProtoMethod(constructor, "start", Start);
env->SetProtoMethod(constructor, "stop", Stop);
Expand Down
2 changes: 1 addition & 1 deletion src/tty_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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, "isRefed", HandleWrap::IsRefed);
env->SetProtoMethod(t, "unrefed", HandleWrap::Unrefed);

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

Expand Down
2 changes: 1 addition & 1 deletion src/udp_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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, "isRefed", HandleWrap::IsRefed);
env->SetProtoMethod(t, "unrefed", HandleWrap::Unrefed);

target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "UDP"), t->GetFunction());
env->set_udp_constructor_function(t->GetFunction());
Expand Down
10 changes: 6 additions & 4 deletions test/parallel/test-handle-wrap-isrefed-tty.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,18 @@ function makeAssert(message) {
strictEqual(actual, expected, message);
};
}
const assert = makeAssert('isRefed() not working on tty_wrap');
const assert = makeAssert('unrefed() 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('isRefed'), true);
assert(tty._handle.isRefed(), true);
assert(Object.getPrototypeOf(tty._handle).hasOwnProperty('unrefed'), true);
assert(tty._handle.unrefed(), false);
tty.unref();
assert(tty._handle.isRefed(), false);
assert(tty._handle.unrefed(), true);
tty._handle.close();
assert(tty._handle.unrefed(), true);
return;
}

Expand Down
74 changes: 40 additions & 34 deletions test/parallel/test-handle-wrap-isrefed.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,86 +12,92 @@ function makeAssert(message) {

// child_process
{
const assert = makeAssert('isRefed() not working on process_wrap');
const assert = makeAssert('unrefed() 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('isRefed'), true);
assert(cp._handle.isRefed(), true);
assert(Object.getPrototypeOf(cp._handle).hasOwnProperty('unrefed'), true);
assert(cp._handle.unrefed(), false);
cp.unref();
assert(cp._handle.isRefed(), false);
assert(cp._handle.unrefed(), true);
cp.ref();
assert(cp._handle.isRefed(), true);
cp.unref();
assert(cp._handle.unrefed(), false);
cp._handle.close();
assert(cp._handle.unrefed(), false);
}


// dgram
{
const assert = makeAssert('isRefed() not working on udp_wrap');
const assert = makeAssert('unrefed() not working on udp_wrap');
const dgram = require('dgram');

const sock4 = dgram.createSocket('udp4');
assert(Object.getPrototypeOf(sock4._handle).hasOwnProperty('isRefed'), true);
assert(sock4._handle.isRefed(), true);
assert(Object.getPrototypeOf(sock4._handle).hasOwnProperty('unrefed'), true);
assert(sock4._handle.unrefed(), false);
sock4.unref();
assert(sock4._handle.isRefed(), false);
assert(sock4._handle.unrefed(), true);
sock4.ref();
assert(sock4._handle.isRefed(), true);
sock4.unref();
assert(sock4._handle.unrefed(), false);
sock4._handle.close();
assert(sock4._handle.unrefed(), false);

const sock6 = dgram.createSocket('udp6');
assert(Object.getPrototypeOf(sock6._handle).hasOwnProperty('isRefed'), true);
assert(sock6._handle.isRefed(), true);
assert(Object.getPrototypeOf(sock6._handle).hasOwnProperty('unrefed'), true);
assert(sock6._handle.unrefed(), false);
sock6.unref();
assert(sock6._handle.isRefed(), false);
assert(sock6._handle.unrefed(), true);
sock6.ref();
assert(sock6._handle.isRefed(), true);
sock6.unref();
assert(sock6._handle.unrefed(), false);
sock6._handle.close();
assert(sock6._handle.unrefed(), false);
}


// pipe
{
const assert = makeAssert('isRefed() not working on pipe_wrap');
const assert = makeAssert('unrefed() not working on pipe_wrap');
const Pipe = process.binding('pipe_wrap').Pipe;
const handle = new Pipe();
assert(Object.getPrototypeOf(handle).hasOwnProperty('isRefed'), true);
assert(handle.isRefed(), true);
assert(Object.getPrototypeOf(handle).hasOwnProperty('unrefed'), true);
assert(handle.unrefed(), false);
handle.unref();
assert(handle.isRefed(), false);
assert(handle.unrefed(), true);
handle.ref();
assert(handle.isRefed(), true);
handle.unref();
assert(handle.unrefed(), false);
handle.close();
assert(handle.unrefed(), false);
}


// tcp
{
const assert = makeAssert('isRefed() not working on tcp_wrap');
const assert = makeAssert('unrefed() not working on tcp_wrap');
const net = require('net');
const server = net.createServer(() => {}).listen(common.PORT);
assert(Object.getPrototypeOf(server._handle).hasOwnProperty('isRefed'), true);
assert(server._handle.isRefed(), true);
assert(Object.getPrototypeOf(server._handle).hasOwnProperty('unrefed'), true);
assert(server._handle.unrefed(), false);
assert(server._unref, false);
server.unref();
assert(server._handle.isRefed(), false);
assert(server._handle.unrefed(), true);
assert(server._unref, true);
server.ref();
assert(server._handle.isRefed(), true);
assert(server._handle.unrefed(), false);
assert(server._unref, false);
server.unref();
server._handle.close();
assert(server._handle.unrefed(), false);
}


// timers
{
const assert = makeAssert('isRefed() not working on timer_wrap');
const assert = makeAssert('unrefed() not working on timer_wrap');
const timer = setTimeout(() => {}, 500);
timer.unref();
assert(Object.getPrototypeOf(timer._handle).hasOwnProperty('isRefed'), true);
assert(timer._handle.isRefed(), false);
assert(Object.getPrototypeOf(timer._handle).hasOwnProperty('unrefed'), true);
assert(timer._handle.unrefed(), true);
timer.ref();
assert(timer._handle.isRefed(), true);
timer.unref();
assert(timer._handle.unrefed(), false);
timer.close();
assert(timer._handle.unrefed(), false);
}

0 comments on commit 2000072

Please sign in to comment.