deasync
deasync copied to clipboard
Match tick callback to Node's logic
Node calls the tick callback from two places: MakeCallback and AsyncWrap::MakeCallback.
Originally, I thought maybe MakeCallback could be used within deasync.cc to gain the same behavior with purely public APIs. My goal was to remove the the use of the private _tick<?Domain>Callback, but invocation of tick_callback_function is actually blocked because node will always be in a nested AsyncCallbackScope for both MakeCallback and AsyncWrap::MakeCallback. Initial code execution begins in such a context, and all future callbacks are done via subclasses of AsyncWrap, so their MakeCallback invocation always puts them back in an AsyncCallbackScope.
So since it wasn't possible to remove _tick<?Domain>Callback, I figured it would at least make sense to mirror how Node chooses between these two functions.
When domain is required it sets up domain use which changes the tick_callback_function in the Environment and also adds a domain property to process. So at the very least, I think it makes sense to invoke the callback that Node would be invoking once no longer in a nested AsyncCallbackScope.
While this doesn't really change much today since _tickCallback and _tickDomainCallback functions are nearly identical (and the domain version is graceful with the absence of a domain), it does better future-proof the code a little (especially since domains are deprecated).
Thx for the pr. The change appears to be innocuous, but does it fix any issues you experienced, or it's just in order to match the 2 callback APIs node provided? It would be nice to identify a bug it can fix to justify the benefit outweighs the risk of causing existing dependencies to break.
No bug fix… just for matching consistency with Node.
You could incorporate this and bump to 0.2.0 as a semver "breaking change" for ^ ranges.
Fwiw, process._tickDomainCallback doesn’t exist anymore in Node 9.3.0+ … I guess if you want a safe way to run the tick queue at a certain point, you can do a dummy MakeCallback from the native binding instead?