From 7c84f19455a29c4c132935aacc3ce1628fc38ac3 Mon Sep 17 00:00:00 2001 From: Kyle Farnung Date: Wed, 10 Jan 2018 15:18:52 -0800 Subject: [PATCH 01/13] test: remove orphaned entries from status PR-URL: https://github.com/nodejs/node/pull/18092 Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Rich Trott Reviewed-By: Gireesh Punathil Reviewed-By: Daniel Bevenius --- test/known_issues/known_issues.status | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/known_issues/known_issues.status b/test/known_issues/known_issues.status index 46c8ed32741..e21913e232c 100644 --- a/test/known_issues/known_issues.status +++ b/test/known_issues/known_issues.status @@ -7,8 +7,6 @@ prefix known_issues [true] # This section applies to all platforms [$system==win32] -test-stdout-buffer-flush-on-exit: SKIP -test-cluster-disconnect-handles: SKIP [$system==linux] From 078b7a2a65599ac6bc7e16e18de482556259d0db Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 10 Jan 2018 09:48:21 -0800 Subject: [PATCH 02/13] doc: update pushStream docs to use err first Refs: https://github.com/nodejs/node/pull/17406#issuecomment-356661798 PR-URL: https://github.com/nodejs/node/pull/18088 Reviewed-By: Anna Henningsen Reviewed-By: Matteo Collina Reviewed-By: Luigi Pinca Reviewed-By: Daniel Bevenius --- doc/api/http2.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/api/http2.md b/doc/api/http2.md index 09bce9d144d..c008a130915 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -1174,14 +1174,16 @@ added: v8.4.0 * Returns: {undefined} Initiates a push stream. The callback is invoked with the new `Http2Stream` -instance created for the push stream. +instance created for the push stream passed as the second argument, or an +`Error` passed as the first argument. ```js const http2 = require('http2'); const server = http2.createServer(); server.on('stream', (stream) => { stream.respond({ ':status': 200 }); - stream.pushStream({ ':path': '/' }, (pushStream) => { + stream.pushStream({ ':path': '/' }, (err, pushStream) => { + if (err) throw err; pushStream.respond({ ':status': 200 }); pushStream.end('some pushed data'); }); From 47a282293f62813a88b4c4ba18bc5e5246a6515c Mon Sep 17 00:00:00 2001 From: James M Snell Date: Thu, 11 Jan 2018 10:05:15 -0800 Subject: [PATCH 03/13] doc: fix s/rstStream/close in example PR-URL: https://github.com/nodejs/node/pull/18088 Reviewed-By: Anna Henningsen Reviewed-By: Matteo Collina Reviewed-By: Luigi Pinca Reviewed-By: Daniel Bevenius --- doc/api/http2.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/http2.md b/doc/api/http2.md index c008a130915..f07c5fb0917 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -1015,7 +1015,7 @@ const { NGHTTP2_CANCEL } = http2.constants; const req = client.request({ ':path': '/' }); // Cancel the stream if there's no activity after 5 seconds -req.setTimeout(5000, () => req.rstStream(NGHTTP2_CANCEL)); +req.setTimeout(5000, () => req.close(NGHTTP2_CANCEL)); ``` #### http2stream.state From 20fe04f113fb81423585077265d3026584989232 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 8 Jan 2018 13:32:30 -0800 Subject: [PATCH 04/13] perf_hooks,http2: add performance.clear() Add missing clear() method to `perf_hooks.performance` to remove the entries from the master timeline to prevent that from being a memory leak. PR-URL: https://github.com/nodejs/node/pull/18046 Reviewed-By: Matteo Collina --- doc/api/perf_hooks.md | 8 ++++++++ lib/perf_hooks.js | 4 ++++ test/parallel/test-http2-perf_hooks.js | 10 +++++++++- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/doc/api/perf_hooks.md b/doc/api/perf_hooks.md index 5ce94624cb9..3f2cc506f31 100644 --- a/doc/api/perf_hooks.md +++ b/doc/api/perf_hooks.md @@ -29,6 +29,14 @@ added: v8.5.0 The `Performance` provides access to performance metric data. A single instance of this class is provided via the `performance` property. +### performance.clearEntries(name) + + +Remove all performance entry objects with `entryType` equal to `name` from the +Performance Timeline. + ### performance.clearFunctions([name]) + +* Value: {[Headers Object][]} + +An object containing the outbound headers sent for this `Http2Stream`. + +#### http2stream.sentInfoHeaders + + +* Value: {[Headers Object][]\[\]} + +An array of objects containing the outbound informational (additional) headers +sent for this `Http2Stream`. + +#### http2stream.sentTrailers + + +* Value: {[Headers Object][]} + +An object containing the outbound trailers sent for this this `HttpStream`. + #### http2stream.session * `options` {Object} Required. Supports the following properties: - * `port` {Integer} + * `port` {integer} * `address` {string} * `exclusive` {boolean} * `callback` {Function} @@ -390,7 +390,7 @@ packets may be sent to a local interface's broadcast address. added: v8.6.0 --> -* `multicastInterface` {String} +* `multicastInterface` {string} *Note: All references to scope in this section are referring to [IPv6 Zone Indices][], which are defined by [RFC 4007][]. In string form, an IP diff --git a/doc/api/fs.md b/doc/api/fs.md index e1dabde956c..f2430b0da09 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -1334,7 +1334,7 @@ deprecated: v1.0.0 * `path` {string|Buffer|URL} * `callback` {Function} - * `exists` {Boolean} + * `exists` {boolean} Test whether or not the given path exists by checking with the file system. Then call the `callback` argument with either true or false. Example: diff --git a/doc/api/net.md b/doc/api/net.md index 03ed3d5cbc3..b213829ffd3 100644 --- a/doc/api/net.md +++ b/doc/api/net.md @@ -281,7 +281,7 @@ server.listen({ added: v0.1.90 --> -* `path` {String} Path the server should listen to. See +* `path` {string} Path the server should listen to. See [Identifying paths for IPC connections][]. * `backlog` {number} Common parameter of [`server.listen()`][] functions. * `callback` {Function} Common parameter of [`server.listen()`][] functions. diff --git a/doc/guides/using-internal-errors.md b/doc/guides/using-internal-errors.md index 3ae11e901a6..90962757bb0 100644 --- a/doc/guides/using-internal-errors.md +++ b/doc/guides/using-internal-errors.md @@ -119,8 +119,8 @@ likely be required. ### Class: errors.Error(key[, args...]) -* `key` {String} The static error identifier -* `args...` {Any} Zero or more optional arguments +* `key` {string} The static error identifier +* `args...` {any} Zero or more optional arguments ```js const errors = require('internal/errors'); @@ -139,8 +139,8 @@ The `myError` object will have a `code` property equal to the `key` and a ### Class: errors.TypeError(key[, args...]) -* `key` {String} The static error identifier -* `args...` {Any} Zero or more optional arguments +* `key` {string} The static error identifier +* `args...` {any} Zero or more optional arguments ```js const errors = require('internal/errors'); @@ -159,8 +159,8 @@ The `myError` object will have a `code` property equal to the `key` and a ### Class: errors.RangeError(key[, args...]) -* `key` {String} The static error identifier -* `args...` {Any} Zero or more optional arguments +* `key` {string} The static error identifier +* `args...` {any} Zero or more optional arguments ```js const errors = require('internal/errors'); @@ -179,8 +179,8 @@ The `myError` object will have a `code` property equal to the `key` and a ### Method: errors.message(key, args) -* `key` {String} The static error identifier +* `key` {string} The static error identifier * `args` {Array} Zero or more optional arguments passed as an Array -* Returns: {String} +* Returns: {string} Returns the formatted error message string for the given `key`. From c3fde98d4d2d920ce3df10a7668aa36bcfb49206 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Fri, 12 Jan 2018 02:07:18 +0200 Subject: [PATCH 09/13] doc: capitalize non-primitive types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/18111 Reviewed-By: Сковорода Никита Андреевич Reviewed-By: James M Snell Reviewed-By: Colin Ihrig --- doc/api/assert.md | 4 ++-- doc/api/cluster.md | 2 +- doc/api/http2.md | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/api/assert.md b/doc/api/assert.md index 75477516bfa..1f49f9ca36c 100644 --- a/doc/api/assert.md +++ b/doc/api/assert.md @@ -375,7 +375,7 @@ added: v0.1.21 * `expected` {any} * `message` {any} **Default:** `'Failed'` * `operator` {string} **Default:** '!=' -* `stackStartFunction` {function} **Default:** `assert.fail` +* `stackStartFunction` {Function} **Default:** `assert.fail` Throws an `AssertionError`. If `message` is falsy, the error message is set as the values of `actual` and `expected` separated by the provided `operator`. If @@ -719,7 +719,7 @@ changes: description: The `error` parameter can now be an arrow function. --> * `block` {Function} -* `error` {RegExp|Function|object} +* `error` {RegExp|Function|Object} * `message` {any} Expects the function `block` to throw an error. diff --git a/doc/api/cluster.md b/doc/api/cluster.md index 016fdf6c185..6ef451cb9af 100644 --- a/doc/api/cluster.md +++ b/doc/api/cluster.md @@ -718,7 +718,7 @@ changes: `'ipc'` entry. When this option is provided, it overrides `silent`. * `uid` {number} Sets the user identity of the process. (See setuid(2).) * `gid` {number} Sets the group identity of the process. (See setgid(2).) - * `inspectPort` {number|function} Sets inspector port of worker. + * `inspectPort` {number|Function} Sets inspector port of worker. This can be a number, or a function that takes no arguments and returns a number. By default each worker gets its own port, incremented from the master's `process.debugPort`. diff --git a/doc/api/http2.md b/doc/api/http2.md index e842ec1c180..fa75f7be5f7 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -1232,7 +1232,7 @@ added: v8.4.0 * `options` {Object} * `endStream` {boolean} Set to `true` to indicate that the response will not include payload data. - * `getTrailers` {function} Callback function invoked to collect trailer + * `getTrailers` {Function} Callback function invoked to collect trailer headers. * Returns: {undefined} From 8479606a24ca12212b75e0febf92925b71ef8d50 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Fri, 5 Jan 2018 15:40:47 +0100 Subject: [PATCH 10/13] async_hooks,http: set HTTPParser trigger to socket This allows more easy tracking of where HTTP requests come from. Before this change the HTTPParser would have the HTTPServer as the triggerAsyncId. The HTTPParser will still have the executionAsyncId set to the HTTP Server so that information is still directly available. Indirectly, the TCP socket itself also has its triggerAsyncId set to the TCP Server. PR-URL: https://github.com/nodejs/node/pull/18003 Reviewed-By: Anna Henningsen Reviewed-By: Anatoli Papirovski Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Jon Moss Reviewed-By: Daijiro Wachi --- lib/_http_server.js | 28 ++++++++++----- lib/internal/async_hooks.js | 12 ++++++- test/async-hooks/test-graph.http.js | 56 +++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 10 deletions(-) create mode 100644 test/async-hooks/test-graph.http.js diff --git a/lib/_http_server.js b/lib/_http_server.js index 5857e43d79c..dee0de74bc7 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -37,6 +37,10 @@ const { } = require('_http_common'); const { OutgoingMessage } = require('_http_outgoing'); const { outHeadersKey, ondrain } = require('internal/http'); +const { + defaultTriggerAsyncIdScope, + getOrSetAsyncId +} = require('internal/async_hooks'); const errors = require('internal/errors'); const Buffer = require('buffer').Buffer; @@ -292,6 +296,12 @@ Server.prototype.setTimeout = function setTimeout(msecs, callback) { function connectionListener(socket) { + defaultTriggerAsyncIdScope( + getOrSetAsyncId(socket), connectionListenerInternal, this, socket + ); +} + +function connectionListenerInternal(server, socket) { debug('SERVER new http connection'); httpSocketSetup(socket); @@ -299,13 +309,13 @@ function connectionListener(socket) { // Ensure that the server property of the socket is correctly set. // See https://github.com/nodejs/node/issues/13435 if (socket.server === null) - socket.server = this; + socket.server = server; // If the user has added a listener to the server, // request, or response, then it's their responsibility. // otherwise, destroy on timeout by default - if (this.timeout && typeof socket.setTimeout === 'function') - socket.setTimeout(this.timeout); + if (server.timeout && typeof socket.setTimeout === 'function') + socket.setTimeout(server.timeout); socket.on('timeout', socketOnTimeout); var parser = parsers.alloc(); @@ -315,8 +325,8 @@ function connectionListener(socket) { parser.incoming = null; // Propagate headers limit from server instance to parser - if (typeof this.maxHeadersCount === 'number') { - parser.maxHeaderPairs = this.maxHeadersCount << 1; + if (typeof server.maxHeadersCount === 'number') { + parser.maxHeaderPairs = server.maxHeadersCount << 1; } else { // Set default value because parser may be reused from FreeList parser.maxHeaderPairs = 2000; @@ -336,8 +346,8 @@ function connectionListener(socket) { outgoingData: 0, keepAliveTimeoutSet: false }; - state.onData = socketOnData.bind(undefined, this, socket, parser, state); - state.onEnd = socketOnEnd.bind(undefined, this, socket, parser, state); + state.onData = socketOnData.bind(undefined, server, socket, parser, state); + state.onEnd = socketOnEnd.bind(undefined, server, socket, parser, state); state.onClose = socketOnClose.bind(undefined, socket, state); state.onDrain = socketOnDrain.bind(undefined, socket, state); socket.on('data', state.onData); @@ -345,7 +355,7 @@ function connectionListener(socket) { socket.on('end', state.onEnd); socket.on('close', state.onClose); socket.on('drain', state.onDrain); - parser.onIncoming = parserOnIncoming.bind(undefined, this, socket, state); + parser.onIncoming = parserOnIncoming.bind(undefined, server, socket, state); // We are consuming socket, so it won't get any actual data socket.on('resume', onSocketResume); @@ -364,7 +374,7 @@ function connectionListener(socket) { } } parser[kOnExecute] = - onParserExecute.bind(undefined, this, socket, parser, state); + onParserExecute.bind(undefined, server, socket, parser, state); socket._paused = false; } diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 2c43ccc3138..f1c98f13ac0 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -26,7 +26,7 @@ const async_wrap = process.binding('async_wrap'); * It has a fixed size, so if that is exceeded, calls to the native * side are used instead in pushAsyncIds() and popAsyncIds(). */ -const { async_hook_fields, async_id_fields } = async_wrap; +const { async_id_symbol, async_hook_fields, async_id_fields } = async_wrap; // Store the pair executionAsyncId and triggerAsyncId in a std::stack on // Environment::AsyncHooks::ids_stack_ tracks the resource responsible for the // current execution stack. This is unwound as each resource exits. In the case @@ -248,6 +248,15 @@ function newUid() { return ++async_id_fields[kAsyncIdCounter]; } +function getOrSetAsyncId(object) { + if (object.hasOwnProperty(async_id_symbol)) { + return object[async_id_symbol]; + } + + return object[async_id_symbol] = newUid(); +} + + // Return the triggerAsyncId meant for the constructor calling it. It's up to // the user to safeguard this call and make sure it's zero'd out when the // constructor is complete. @@ -378,6 +387,7 @@ module.exports = { disableHooks, // Internal Embedder API newUid, + getOrSetAsyncId, getDefaultTriggerAsyncId, defaultTriggerAsyncIdScope, emitInit: emitInitScript, diff --git a/test/async-hooks/test-graph.http.js b/test/async-hooks/test-graph.http.js new file mode 100644 index 00000000000..5c0c99f408c --- /dev/null +++ b/test/async-hooks/test-graph.http.js @@ -0,0 +1,56 @@ +'use strict'; + +const common = require('../common'); +const initHooks = require('./init-hooks'); +const verifyGraph = require('./verify-graph'); +const http = require('http'); + +const hooks = initHooks(); +hooks.enable(); + +const server = http.createServer(common.mustCall(function(req, res) { + res.end(); + this.close(common.mustCall()); +})); +server.listen(0, common.mustCall(function() { + http.get(`http://127.0.0.1:${server.address().port}`, common.mustCall()); +})); + +process.on('exit', function() { + hooks.disable(); + + verifyGraph( + hooks, + [ { type: 'TCPSERVERWRAP', + id: 'tcpserver:1', + triggerAsyncId: null }, + { type: 'TCPWRAP', id: 'tcp:1', triggerAsyncId: 'tcpserver:1' }, + { type: 'TCPCONNECTWRAP', + id: 'tcpconnect:1', + triggerAsyncId: 'tcp:1' }, + { type: 'HTTPPARSER', + id: 'httpparser:1', + triggerAsyncId: 'tcpserver:1' }, + { type: 'HTTPPARSER', + id: 'httpparser:2', + triggerAsyncId: 'tcpserver:1' }, + { type: 'TCPWRAP', id: 'tcp:2', triggerAsyncId: 'tcpserver:1' }, + { type: 'Timeout', id: 'timeout:1', triggerAsyncId: 'tcp:2' }, + { type: 'TIMERWRAP', id: 'timer:1', triggerAsyncId: 'tcp:2' }, + { type: 'HTTPPARSER', + id: 'httpparser:3', + triggerAsyncId: 'tcp:2' }, + { type: 'HTTPPARSER', + id: 'httpparser:4', + triggerAsyncId: 'tcp:2' }, + { type: 'Timeout', + id: 'timeout:2', + triggerAsyncId: 'httpparser:4' }, + { type: 'TIMERWRAP', + id: 'timer:2', + triggerAsyncId: 'httpparser:4' }, + { type: 'SHUTDOWNWRAP', + id: 'shutdown:1', + triggerAsyncId: 'tcp:2' } ] + ); +}); From c893f488c306171f5ecac5ba2955958048abe4c1 Mon Sep 17 00:00:00 2001 From: Leko Date: Fri, 12 Jan 2018 00:48:17 +0900 Subject: [PATCH 11/13] doc: add Leko to collaborators PR-URL: https://github.com/nodejs/node/pull/18117 Reviewed-By: Joyee Cheung Reviewed-By: Vse Mozhet Byt Reviewed-By: Colin Ihrig Reviewed-By: Yuta Hiroto Reviewed-By: Daijiro Wachi Reviewed-By: Richard Lau --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 85a27f549a5..e9eb26e7342 100644 --- a/README.md +++ b/README.md @@ -405,6 +405,8 @@ For more information about the governance of the Node.js project, see **Kunal Pathak** <kunal.pathak@microsoft.com> * [lance](https://github.com/lance) - **Lance Ball** <lball@redhat.com> +* [Leko](https://github.com/Leko) - +**Shingo Inoue** <leko.noor@gmail.com> (he/him) * [lpinca](https://github.com/lpinca) - **Luigi Pinca** <luigipinca@gmail.com> (he/him) * [lucamaraschi](https://github.com/lucamaraschi) - From e8c491a801191bd6e6244195237adc377a4f755f Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 9 Jan 2018 19:34:11 +0800 Subject: [PATCH 12/13] doc: prefer make test-only when verifying the build PR-URL: https://github.com/nodejs/node/pull/18061 Reviewed-By: James M Snell Reviewed-By: Jeremiah Senkpiel Reviewed-By: Khaidi Chu Reviewed-By: Luigi Pinca Reviewed-By: Daniel Bevenius --- BUILDING.md | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/BUILDING.md b/BUILDING.md index fbf6b9cd964..296296e558d 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -94,7 +94,7 @@ Depending on host platform, the selection of toolchains may vary. ### Unix/macOS -Prerequisites: +#### Prerequisites * `gcc` and `g++` 4.9.4 or newer, or * `clang` and `clang++` 3.4.2 or newer (macOS: latest Xcode Command Line Tools) @@ -120,6 +120,8 @@ directory and the symbolic `node` link in the project's root directory. On FreeBSD and OpenBSD, you may also need: * libexecinfo +#### Building Node.js + To build Node.js: ```console @@ -138,13 +140,26 @@ for more information. Note that the above requires that `python` resolve to Python 2.6 or 2.7 and not a newer version. -To run the tests: +#### Running Tests + +To verify the build: + +```console +$ make test-only +``` + +At this point, you are ready to make code changes and re-run the tests. + +If you are running tests prior to submitting a Pull Request, the recommended +command is: ```console $ make test ``` -At this point you are ready to make code changes and re-run the tests! +`make test` does a full check on the codebase, including running linters and +documentation tests. + Optionally, continue below. To run the tests and generate code coverage reports: @@ -166,6 +181,8 @@ reports: $ make coverage-clean ``` +#### Building the documentation + To build the documentation: This will build Node.js first (if necessary) and then use it to build the docs: From 1385e1bc63665b8c226e9f7bc8c46a9263195efc Mon Sep 17 00:00:00 2001 From: zhangzifa Date: Mon, 14 Aug 2017 09:59:15 +0800 Subject: [PATCH 13/13] timers: setInterval interval includes cb duration setInterval callback should be scheduled on the interval Fixes: https://github.com/nodejs/node/issues/7346 PR-URL: https://github.com/nodejs/node/pull/14815 Fixes: https://github.com/nodejs/node/issues/7346 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Jeremiah Senkpiel Reviewed-By: Matteo Collina Reviewed-By: Ruben Bridgewater Reviewed-By: Michael Dawson --- lib/timers.js | 23 +++++++++++++++---- ...-setInterval-excludes-callback-duration.js | 18 +++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 test/sequential/test-timers-setInterval-excludes-callback-duration.js diff --git a/lib/timers.js b/lib/timers.js index 643731b105e..8a74bcf2de7 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -167,11 +167,15 @@ exports._unrefActive = function(item) { // Appends a timer onto the end of an existing timers list, or creates a new // TimerWrap backed list if one does not already exist for the specified timeout // duration. -function insert(item, unrefed) { +function insert(item, unrefed, start) { const msecs = item._idleTimeout; if (msecs < 0 || msecs === undefined) return; - item._idleStart = TimerWrap.now(); + if (typeof start === 'number') { + item._idleStart = start; + } else { + item._idleStart = TimerWrap.now(); + } const lists = unrefed === true ? unrefedLists : refedLists; @@ -446,16 +450,17 @@ function ontimeout(timer) { var args = timer._timerArgs; if (typeof timer._onTimeout !== 'function') return promiseResolve(timer._onTimeout, args[0]); + const start = TimerWrap.now(); if (!args) timer._onTimeout(); else Reflect.apply(timer._onTimeout, timer, args); if (timer._repeat) - rearm(timer); + rearm(timer, start); } -function rearm(timer) { +function rearm(timer, start) { // // Do not re-arm unenroll'd or closed timers. if (timer._idleTimeout === -1) return; @@ -464,7 +469,15 @@ function rearm(timer) { timer._handle.start(timer._repeat); } else { timer._idleTimeout = timer._repeat; - active(timer); + + const duration = TimerWrap.now() - start; + if (duration >= timer._repeat) { + // If callback duration >= timer._repeat, + // add 1 ms to avoid blocking eventloop + insert(timer, false, start + duration - timer._repeat + 1); + } else { + insert(timer, false, start); + } } } diff --git a/test/sequential/test-timers-setInterval-excludes-callback-duration.js b/test/sequential/test-timers-setInterval-excludes-callback-duration.js new file mode 100644 index 00000000000..662a5055b70 --- /dev/null +++ b/test/sequential/test-timers-setInterval-excludes-callback-duration.js @@ -0,0 +1,18 @@ +'use strict'; +const common = require('../common'); +const Timer = process.binding('timer_wrap').Timer; +const assert = require('assert'); + +let cntr = 0; +let first, second; +const t = setInterval(() => { + common.busyLoop(50); + cntr++; + if (cntr === 1) { + first = Timer.now(); + } else if (cntr === 2) { + second = Timer.now(); + assert(Math.abs(second - first - 100) < 10); + clearInterval(t); + } +}, 100);