Skip to content

Commit

Permalink
src: return early if nextTickQueue is empty
Browse files Browse the repository at this point in the history
This brings the node::MakeCallback and node::AsyncWrap::MakeCallback
implementations into alignment in that they return early if the
nextTickQueue is empty after processing the MicrotaskQueue.

Include test to make sure early return happens. Test has text explaining
the conditions for the test to pass, since it relies on internal
mechanisms that aren't guaranteed in the future.

PR-URL: #10274
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
trevnorris authored and evanlucas committed Jan 3, 2017
1 parent 586967a commit 5e5b1f8
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1274,6 +1274,7 @@ Local<Value> MakeCallback(Environment* env,

if (tick_info->length() == 0) {
tick_info->set_index(0);
return ret;
}

if (env->tick_callback_function()->Call(process, 0, nullptr).IsEmpty()) {
Expand Down
32 changes: 32 additions & 0 deletions test/parallel/test-no-enter-tickcallback.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
'use strict';

const common = require('../common');
const assert = require('assert');
var allsGood = false;
var cntr = 0;

process.on('exit', () => {
assert.ok(cntr > 0, '_tickDomainCallback was never called');
});

/**
* This test relies upon the following internals to work as specified:
* - require('domain') causes node::Environment::set_tick_callback_function()
* to use process._tickDomainCallback() to process the nextTickQueue;
* replacing process._tickCallback().
* - setImmediate() uses node::MakeCallback() instead of
* node::AsyncWrap::MakeCallback(). Otherwise the test will always pass.
* Have not found a way to verify that node::MakeCallback() is used.
*/
process._tickDomainCallback = function _tickDomainCallback() {
assert.ok(allsGood, '_tickDomainCallback should not have been called');
cntr++;
};

setImmediate(common.mustCall(() => {
require('domain');
setImmediate(common.mustCall(() => setImmediate(common.mustCall(() => {
allsGood = true;
process.nextTick(() => {});
}))));
}));

0 comments on commit 5e5b1f8

Please sign in to comment.