diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 7d8ec2e772002e..cf063e5f9b7f05 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -52,10 +52,12 @@ const { } = require('internal/http2/util'); const { - _unrefActive, - enroll, - unenroll -} = require('timers'); + kTimeout, + setUnrefTimeout, + validateTimerDuration +} = require('internal/timers'); + +const { _unrefActive } = require('timers'); const { ShutdownWrap, WriteWrap } = process.binding('stream_wrap'); const { constants } = binding; @@ -280,8 +282,8 @@ function onStreamClose(code, hasData) { ` [has data? ${hasData}]`); if (!stream.closed) { - // Unenroll from timeouts - unenroll(stream); + // Clear timeout and remove timeout listeners + stream.setTimeout(0); stream.removeAllListeners('timeout'); // Set the state flags @@ -788,6 +790,7 @@ class Http2Session extends EventEmitter { this[kType] = type; this[kProxySocket] = null; this[kSocket] = socket; + this[kTimeout] = null; // Do not use nagle's algorithm if (typeof socket.setNoDelay === 'function') @@ -828,7 +831,7 @@ class Http2Session extends EventEmitter { [kUpdateTimer]() { if (this.destroyed) return; - _unrefActive(this); + if (this[kTimeout]) _unrefActive(this[kTimeout]); } // Sets the id of the next stream to be created by this Http2Session. @@ -1019,7 +1022,7 @@ class Http2Session extends EventEmitter { state.flags |= SESSION_FLAGS_DESTROYED; // Clear timeout and remove timeout listeners - unenroll(this); + this.setTimeout(0); this.removeAllListeners('timeout'); // Destroy any pending and open streams @@ -1322,6 +1325,8 @@ class Http2Stream extends Duplex { this[kSession] = session; session[kState].pendingStreams.add(this); + this[kTimeout] = null; + this[kState] = { flags: STREAM_FLAGS_PENDING, rstCode: NGHTTP2_NO_ERROR, @@ -1336,9 +1341,10 @@ class Http2Stream extends Duplex { [kUpdateTimer]() { if (this.destroyed) return; - _unrefActive(this); + if (this[kTimeout]) + _unrefActive([kTimeout]); if (this[kSession]) - _unrefActive(this[kSession]); + this[kSession][kUpdateTimer](); } [kInit](id, handle) { @@ -1560,7 +1566,7 @@ class Http2Stream extends Duplex { // Close initiates closing the Http2Stream instance by sending an RST_STREAM // frame to the connected peer. The readable and writable sides of the - // Http2Stream duplex are closed and the timeout timer is unenrolled. If + // Http2Stream duplex are closed and the timeout timer is cleared. If // a callback is passed, it is registered to listen for the 'close' event. // // If the handle and stream ID have not been assigned yet, the close @@ -1577,8 +1583,8 @@ class Http2Stream extends Duplex { if (code < 0 || code > kMaxInt) throw new errors.RangeError('ERR_OUT_OF_RANGE', 'code'); - // Unenroll the timeout. - unenroll(this); + // Clear timeout and remove timeout listeners + this.setTimeout(0); this.removeAllListeners('timeout'); // Close the writable @@ -1637,8 +1643,10 @@ class Http2Stream extends Duplex { handle.destroy(); session[kState].streams.delete(id); } else { - unenroll(this); + // Clear timeout and remove timeout listeners + this.setTimeout(0); this.removeAllListeners('timeout'); + state.flags |= STREAM_FLAGS_CLOSED; abort(this); this.end(); @@ -2216,21 +2224,24 @@ const setTimeout = { value: function(msecs, callback) { if (this.destroyed) return; - if (typeof msecs !== 'number') { - throw new errors.TypeError('ERR_INVALID_ARG_TYPE', - 'msecs', - 'number'); - } + + // Type checking identical to timers.enroll() + msecs = validateTimerDuration(msecs); + + // Attempt to clear an existing timer lear in both cases - + // even if it will be rescheduled we don't want to leak an existing timer. + clearTimeout(this[kTimeout]); + if (msecs === 0) { - unenroll(this); if (callback !== undefined) { if (typeof callback !== 'function') throw new errors.TypeError('ERR_INVALID_CALLBACK'); this.removeListener('timeout', callback); } } else { - enroll(this, msecs); - this[kUpdateTimer](); + this[kTimeout] = setUnrefTimeout(this._onTimeout.bind(this), msecs); + if (this[kSession]) this[kSession][kUpdateTimer](); + if (callback !== undefined) { if (typeof callback !== 'function') throw new errors.TypeError('ERR_INVALID_CALLBACK'); diff --git a/lib/internal/timers.js b/lib/internal/timers.js new file mode 100644 index 00000000000000..e4fbc6e0a363d4 --- /dev/null +++ b/lib/internal/timers.js @@ -0,0 +1,131 @@ +'use strict'; + +const async_wrap = process.binding('async_wrap'); +// Two arrays that share state between C++ and JS. +const { async_hook_fields, async_id_fields } = async_wrap; +const { + getDefaultTriggerAsyncId, + // The needed emit*() functions. + emitInit +} = require('internal/async_hooks'); +// Grab the constants necessary for working with internal arrays. +const { kInit, kAsyncIdCounter } = async_wrap.constants; +// Symbols for storing async id state. +const async_id_symbol = Symbol('asyncId'); +const trigger_async_id_symbol = Symbol('triggerId'); + +const errors = require('internal/errors'); + +// Timeout values > TIMEOUT_MAX are set to 1. +const TIMEOUT_MAX = 2 ** 31 - 1; + +module.exports = { + TIMEOUT_MAX, + kTimeout: Symbol('timeout'), // For hiding Timeouts on other internals. + async_id_symbol, + trigger_async_id_symbol, + Timeout, + setUnrefTimeout, + validateTimerDuration +}; + +// Timer constructor function. +// The entire prototype is defined in lib/timers.js +function Timeout(callback, after, args, isRepeat) { + after *= 1; // coalesce to number or NaN + if (!(after >= 1 && after <= TIMEOUT_MAX)) { + if (after > TIMEOUT_MAX) { + process.emitWarning(`${after} does not fit into` + + ' a 32-bit signed integer.' + + '\nTimeout duration was set to 1.', + 'TimeoutOverflowWarning'); + } + after = 1; // schedule on next tick, follows browser behavior + } + + this._called = false; + this._idleTimeout = after; + this._idlePrev = this; + this._idleNext = this; + this._idleStart = null; + // this must be set to null first to avoid function tracking + // on the hidden class, revisit in V8 versions after 6.2 + this._onTimeout = null; + this._onTimeout = callback; + this._timerArgs = args; + this._repeat = isRepeat ? after : null; + this._destroyed = false; + + this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter]; + this[trigger_async_id_symbol] = getDefaultTriggerAsyncId(); + if (async_hook_fields[kInit] > 0) { + emitInit(this[async_id_symbol], + 'Timeout', + this[trigger_async_id_symbol], + this); + } +} + +var timers; +function getTimers() { + if (timers === undefined) { + timers = require('timers'); + } + return timers; +} + +function setUnrefTimeout(callback, after, arg1, arg2, arg3) { + // Type checking identical to setTimeout() + if (typeof callback !== 'function') { + throw new errors.TypeError('ERR_INVALID_CALLBACK'); + } + + let i, args; + switch (arguments.length) { + // fast cases + case 1: + case 2: + break; + case 3: + args = [arg1]; + break; + case 4: + args = [arg1, arg2]; + break; + default: + args = [arg1, arg2, arg3]; + for (i = 5; i < arguments.length; i++) { + // extend array dynamically, makes .apply run much faster in v6.0.0 + args[i - 2] = arguments[i]; + } + break; + } + + const timer = new Timeout(callback, after, args, false); + getTimers()._unrefActive(timer); + + return timer; +} + +// Type checking used by timers.enroll() and Socket#setTimeout() +function validateTimerDuration(msecs) { + if (typeof msecs !== 'number') { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'msecs', + 'number', msecs); + } + + if (msecs < 0 || !isFinite(msecs)) { + throw new errors.RangeError('ERR_VALUE_OUT_OF_RANGE', 'msecs', + 'a non-negative finite number', msecs); + } + + // Ensure that msecs fits into signed int32 + if (msecs > TIMEOUT_MAX) { + process.emitWarning(`${msecs} does not fit into a 32-bit signed integer.` + + `\nTimer duration was truncated to ${TIMEOUT_MAX}.`, + 'TimeoutOverflowWarning'); + return TIMEOUT_MAX; + } + + return msecs; +} diff --git a/lib/net.js b/lib/net.js index ce87a60ce0e78b..09ebc1b082a065 100644 --- a/lib/net.js +++ b/lib/net.js @@ -57,6 +57,12 @@ var cluster = null; const errnoException = util._errnoException; const exceptionWithHostPort = util._exceptionWithHostPort; +const { + kTimeout, + setUnrefTimeout, + validateTimerDuration +} = require('internal/timers'); + function noop() {} function createHandle(fd, is_server) { @@ -201,6 +207,7 @@ function Socket(options) { this._parent = null; this._host = null; this[kLastWriteQueueSize] = 0; + this[kTimeout] = null; if (typeof options === 'number') options = { fd: options }; // Legacy interface. @@ -272,9 +279,12 @@ function Socket(options) { } util.inherits(Socket, stream.Duplex); +// Refresh existing timeouts. Socket.prototype._unrefTimer = function _unrefTimer() { - for (var s = this; s !== null; s = s._parent) - timers._unrefActive(s); + for (var s = this; s !== null; s = s._parent) { + if (s[kTimeout]) + timers._unrefActive(s[kTimeout]); + } }; @@ -387,14 +397,20 @@ Socket.prototype.read = function(n) { }; Socket.prototype.setTimeout = function(msecs, callback) { + // Type checking identical to timers.enroll() + msecs = validateTimerDuration(msecs); + + // Attempt to clear an existing timer lear in both cases - + // even if it will be rescheduled we don't want to leak an existing timer. + clearTimeout(this[kTimeout]); + if (msecs === 0) { - timers.unenroll(this); if (callback) { this.removeListener('timeout', callback); } } else { - timers.enroll(this, msecs); - timers._unrefActive(this); + this[kTimeout] = setUnrefTimeout(this._onTimeout.bind(this), msecs); + if (callback) { this.once('timeout', callback); } @@ -551,8 +567,9 @@ Socket.prototype._destroy = function(exception, cb) { this.readable = this.writable = false; - for (var s = this; s !== null; s = s._parent) - timers.unenroll(s); + for (var s = this; s !== null; s = s._parent) { + clearTimeout(s[kTimeout]); + } debug('close'); if (this._handle) { diff --git a/lib/timers.js b/lib/timers.js index 479463d2d0ad4c..5539234352b789 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -24,6 +24,7 @@ const async_wrap = process.binding('async_wrap'); const TimerWrap = process.binding('timer_wrap').Timer; const L = require('internal/linkedlist'); +const timerInternals = require('internal/timers'); const internalUtil = require('internal/util'); const { createPromise, promiseResolve } = process.binding('util'); const assert = require('assert'); @@ -44,8 +45,8 @@ const { // Grab the constants necessary for working with internal arrays. const { kInit, kDestroy, kAsyncIdCounter } = async_wrap.constants; // Symbols for storing async id state. -const async_id_symbol = Symbol('asyncId'); -const trigger_async_id_symbol = Symbol('triggerAsyncId'); +const async_id_symbol = timerInternals.async_id_symbol; +const trigger_async_id_symbol = timerInternals.trigger_async_id_symbol; /* This is an Uint32Array for easier sharing with C++ land. */ const scheduledImmediateCount = process._scheduledImmediateCount; @@ -54,8 +55,8 @@ delete process._scheduledImmediateCount; const activateImmediateCheck = process._activateImmediateCheck; delete process._activateImmediateCheck; -// Timeout values > TIMEOUT_MAX are set to 1. -const TIMEOUT_MAX = 2 ** 31 - 1; +// The Timeout class +const Timeout = timerInternals.Timeout; // HOW and WHY the timers implementation works the way it does. @@ -388,29 +389,12 @@ const unenroll = exports.unenroll = function(item) { // This function does not start the timer, see `active()`. // Using existing objects as timers slightly reduces object overhead. exports.enroll = function(item, msecs) { - if (typeof msecs !== 'number') { - throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'msecs', - 'number', msecs); - } - - if (msecs < 0 || !isFinite(msecs)) { - throw new errors.RangeError('ERR_VALUE_OUT_OF_RANGE', 'msecs', - 'a non-negative finite number', msecs); - } + item._idleTimeout = timerInternals.validateTimerDuration(msecs); // if this item was already in a list somewhere // then we should unenroll it from that if (item._idleNext) unenroll(item); - // Ensure that msecs fits into signed int32 - if (msecs > TIMEOUT_MAX) { - process.emitWarning(`${msecs} does not fit into a 32-bit signed integer.` + - `\nTimer duration was truncated to ${TIMEOUT_MAX}.`, - 'TimeoutOverflowWarning'); - msecs = TIMEOUT_MAX; - } - - item._idleTimeout = msecs; L.init(item); }; @@ -446,12 +430,17 @@ function setTimeout(callback, after, arg1, arg2, arg3) { break; } - return new Timeout(callback, after, args, false); + const timeout = new Timeout(callback, after, args, false); + active(timeout); + + return timeout; } setTimeout[internalUtil.promisify.custom] = function(after, value) { const promise = createPromise(); - new Timeout(promise, after, [value], false); + const timeout = new Timeout(promise, after, [value], false); + active(timeout); + return promise; }; @@ -523,7 +512,10 @@ exports.setInterval = function(callback, repeat, arg1, arg2, arg3) { break; } - return new Timeout(callback, repeat, args, true); + const timeout = new Timeout(callback, repeat, args, true); + active(timeout); + + return timeout; }; exports.clearInterval = function(timer) { @@ -534,44 +526,6 @@ exports.clearInterval = function(timer) { }; -function Timeout(callback, after, args, isRepeat) { - after *= 1; // coalesce to number or NaN - if (!(after >= 1 && after <= TIMEOUT_MAX)) { - if (after > TIMEOUT_MAX) { - process.emitWarning(`${after} does not fit into` + - ' a 32-bit signed integer.' + - '\nTimeout duration was set to 1.', - 'TimeoutOverflowWarning'); - } - after = 1; // schedule on next tick, follows browser behavior - } - - this._called = false; - this._idleTimeout = after; - this._idlePrev = this; - this._idleNext = this; - this._idleStart = null; - // this must be set to null first to avoid function tracking - // on the hidden class, revisit in V8 versions after 6.2 - this._onTimeout = null; - this._onTimeout = callback; - this._timerArgs = args; - this._repeat = isRepeat ? after : null; - this._destroyed = false; - - this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter]; - this[trigger_async_id_symbol] = getDefaultTriggerAsyncId(); - if (async_hook_fields[kInit] > 0) { - emitInit(this[async_id_symbol], - 'Timeout', - this[trigger_async_id_symbol], - this); - } - - active(this); -} - - function unrefdHandle() { // Don't attempt to call the callback if it is not a function. if (typeof this.owner._onTimeout === 'function') { diff --git a/node.gyp b/node.gyp index 5d4650e560ccfc..c1c83f6213f262 100644 --- a/node.gyp +++ b/node.gyp @@ -123,6 +123,7 @@ 'lib/internal/repl/await.js', 'lib/internal/socket_list.js', 'lib/internal/test/unicode.js', + 'lib/internal/timers.js', 'lib/internal/tls.js', 'lib/internal/trace_events_async_hooks.js', 'lib/internal/url.js', diff --git a/test/parallel/test-http-client-timeout-on-connect.js b/test/parallel/test-http-client-timeout-on-connect.js index af3b3ef53debc2..928f781e261758 100644 --- a/test/parallel/test-http-client-timeout-on-connect.js +++ b/test/parallel/test-http-client-timeout-on-connect.js @@ -1,7 +1,11 @@ +// Flags: --expose-internals + 'use strict'; + const common = require('../common'); const assert = require('assert'); const http = require('http'); +const { kTimeout } = require('internal/timers'); const server = http.createServer((req, res) => { // This space is intentionally left blank. @@ -13,9 +17,9 @@ server.listen(0, common.localhostIPv4, common.mustCall(() => { req.setTimeout(1); req.on('socket', common.mustCall((socket) => { - assert.strictEqual(socket._idleTimeout, undefined); + assert.strictEqual(socket[kTimeout], null); socket.on('connect', common.mustCall(() => { - assert.strictEqual(socket._idleTimeout, 1); + assert.strictEqual(socket[kTimeout]._idleTimeout, 1); })); })); req.on('timeout', common.mustCall(() => req.abort())); diff --git a/test/parallel/test-http2-compat-socket.js b/test/parallel/test-http2-compat-socket.js index 80c8b1d30d10b3..a54909d3c148ba 100644 --- a/test/parallel/test-http2-compat-socket.js +++ b/test/parallel/test-http2-compat-socket.js @@ -1,3 +1,5 @@ +// Flags: --expose_internals + 'use strict'; const common = require('../common'); @@ -7,6 +9,8 @@ const assert = require('assert'); const h2 = require('http2'); const net = require('net'); +const { kTimeout } = require('internal/timers'); + // Tests behavior of the proxied socket in Http2ServerRequest // & Http2ServerResponse - this proxy socket should mimic the // behavior of http1 but against the http2 api & model @@ -31,7 +35,7 @@ server.on('request', common.mustCall(function(request, response) { assert.strictEqual(request.socket.destroyed, false); request.socket.setTimeout(987); - assert.strictEqual(request.stream.session._idleTimeout, 987); + assert.strictEqual(request.stream.session[kTimeout]._idleTimeout, 987); request.socket.setTimeout(0); common.expectsError(() => request.socket.read(), errMsg); diff --git a/test/parallel/test-http2-socket-proxy.js b/test/parallel/test-http2-socket-proxy.js index 2d90ef7e952a55..1ca34eb451d23c 100644 --- a/test/parallel/test-http2-socket-proxy.js +++ b/test/parallel/test-http2-socket-proxy.js @@ -1,3 +1,5 @@ +// Flags: --expose_internals + 'use strict'; const common = require('../common'); @@ -7,6 +9,8 @@ const assert = require('assert'); const h2 = require('http2'); const net = require('net'); +const { kTimeout } = require('internal/timers'); + // Tests behavior of the proxied socket on Http2Session const errMsg = { @@ -29,7 +33,7 @@ server.on('stream', common.mustCall(function(stream, headers) { assert.strictEqual(typeof socket.address(), 'object'); socket.setTimeout(987); - assert.strictEqual(session._idleTimeout, 987); + assert.strictEqual(session[kTimeout]._idleTimeout, 987); common.expectsError(() => socket.destroy, errMsg); common.expectsError(() => socket.emit, errMsg); @@ -59,9 +63,6 @@ server.on('stream', common.mustCall(function(stream, headers) { stream.end(); - socket.setTimeout = undefined; - assert.strictEqual(session.setTimeout, undefined); - stream.session.on('close', common.mustCall(() => { assert.strictEqual(session.socket, undefined); })); diff --git a/test/parallel/test-http2-timeouts.js b/test/parallel/test-http2-timeouts.js index 083dcaf40c10ad..db5822776aea5e 100644 --- a/test/parallel/test-http2-timeouts.js +++ b/test/parallel/test-http2-timeouts.js @@ -20,7 +20,8 @@ server.on('stream', common.mustCall((stream) => { { code: 'ERR_INVALID_ARG_TYPE', type: TypeError, - message: 'The "msecs" argument must be of type number' + message: + 'The "msecs" argument must be of type number. Received type string' } ); common.expectsError( diff --git a/test/parallel/test-net-socket-timeout.js b/test/parallel/test-net-socket-timeout.js index de4a7ed37ccf20..178e2d994daab0 100644 --- a/test/parallel/test-net-socket-timeout.js +++ b/test/parallel/test-net-socket-timeout.js @@ -36,13 +36,13 @@ const validDelays = [0, 0.001, 1, 1e6]; for (let i = 0; i < nonNumericDelays.length; i++) { assert.throws(function() { s.setTimeout(nonNumericDelays[i], () => {}); - }, TypeError); + }, TypeError, nonNumericDelays[i]); } for (let i = 0; i < badRangeDelays.length; i++) { assert.throws(function() { s.setTimeout(badRangeDelays[i], () => {}); - }, RangeError); + }, RangeError, badRangeDelays[i]); } for (let i = 0; i < validDelays.length; i++) { diff --git a/test/parallel/test-tls-wrap-timeout.js b/test/parallel/test-tls-wrap-timeout.js index d1598ab737f129..6ae2c39c59b8d9 100644 --- a/test/parallel/test-tls-wrap-timeout.js +++ b/test/parallel/test-tls-wrap-timeout.js @@ -1,5 +1,8 @@ +// Flags: --expose_internals + 'use strict'; const common = require('../common'); +const { kTimeout, TIMEOUT_MAX } = require('internal/timers'); if (!common.hasCrypto) common.skip('missing crypto'); @@ -30,13 +33,13 @@ let lastIdleStart; server.listen(0, () => { socket = net.connect(server.address().port, function() { - const s = socket.setTimeout(Number.MAX_VALUE, function() { + const s = socket.setTimeout(TIMEOUT_MAX, function() { throw new Error('timeout'); }); assert.ok(s instanceof net.Socket); - assert.notStrictEqual(socket._idleTimeout, -1); - lastIdleStart = socket._idleStart; + assert.notStrictEqual(socket[kTimeout]._idleTimeout, -1); + lastIdleStart = socket[kTimeout]._idleStart; const tsocket = tls.connect({ socket: socket, @@ -47,6 +50,6 @@ server.listen(0, () => { }); process.on('exit', () => { - assert.strictEqual(socket._idleTimeout, -1); - assert(lastIdleStart < socket._idleStart); + assert.strictEqual(socket[kTimeout]._idleTimeout, -1); + assert(lastIdleStart < socket[kTimeout]._idleStart); });