diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 7d8ec2e772002e..ff5e88f6c65910 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[kSession]) - _unrefActive(this[kSession]); + if (this[kTimeout]) + _unrefActive([kTimeout]); + if (this[kSession] && this[kSession][kTimeout]) + _unrefActive(this[kSession][kTimeout]); } [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/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(