From 8f8db142fa2b2699b30b3827f7c71bff07cb1869 Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Wed, 14 Dec 2016 17:17:10 -0700 Subject: [PATCH] src: return early if nextTickQueue is empty 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: https://github.com/nodejs/node/pull/10274 Reviewed-By: Ben Noordhuis Reviewed-By: Anna Henningsen Reviewed-By: Jeremiah Senkpiel Reviewed-By: Colin Ihrig --- src/node.cc | 1 + test/parallel/test-no-enter-tickcallback.js | 32 +++++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 test/parallel/test-no-enter-tickcallback.js diff --git a/src/node.cc b/src/node.cc index 1302dfed169a18..f3d0f03568a79a 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1268,6 +1268,7 @@ Local 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()) { diff --git a/test/parallel/test-no-enter-tickcallback.js b/test/parallel/test-no-enter-tickcallback.js new file mode 100644 index 00000000000000..e06628628955be --- /dev/null +++ b/test/parallel/test-no-enter-tickcallback.js @@ -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(() => {}); + })))); +}));