◐ Shell
clean mode source ↗

timers: use HandleWrap::Unrefed by Fishrock123 · Pull Request #6381 · nodejs/node

@Fishrock123 added confirmed-bug

Issues with confirmed bugs.

c++

Issues and PRs that require attention from people who are familiar with C++.

timers

Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.

labels

Apr 25, 2016

@Fishrock123

@Fishrock123

Fishrock123 added a commit to Fishrock123/node that referenced this pull request

May 11, 2016
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: nodejs#6395
Refs: nodejs#6204
Refs: nodejs#6401
Refs: nodejs#6382
Fixes: nodejs#6381

PR-URL: nodejs#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

Fishrock123 added a commit to Fishrock123/node that referenced this pull request

May 11, 2016

evanlucas pushed a commit that referenced this pull request

May 17, 2016
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

evanlucas pushed a commit that referenced this pull request

May 17, 2016
Rename slightly to HasRef() at bnoordhuis’ request.
Better reflects what we actually do for this check.

Refs: #6395
Refs: #6204
Refs: #6401
Refs: #6382
Refs: #6381

PR-URL: #6546
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>