diff options
| -rw-r--r-- | lib/internal/http2/core.js | 82 | ||||
| -rw-r--r-- | test/parallel/test-http2-session-timeout.js | 41 |
2 files changed, 82 insertions, 41 deletions
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 1ca1a89c699..9e698d8874f 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -150,8 +150,8 @@ function emit() { // event. If the stream is not new, emit the 'headers' event to pass // the block of headers on. function onSessionHeaders(id, cat, flags, headers) { - _unrefActive(this); const owner = this[kOwner]; + _unrefActive(owner); debug(`[${sessionName(owner[kType])}] headers were received on ` + `stream ${id}: ${cat}`); const streams = owner[kState].streams; @@ -265,7 +265,7 @@ function onSessionStreamClose(id, code) { const stream = owner[kState].streams.get(id); if (stream === undefined) return; - _unrefActive(this); + _unrefActive(owner); // Set the rst state for the stream abort(stream); const state = stream[kState]; @@ -287,14 +287,16 @@ function afterFDClose(err) { // Called when an error event needs to be triggered function onSessionError(error) { - _unrefActive(this); - process.nextTick(() => this[kOwner].emit('error', error)); + const owner = this[kOwner]; + _unrefActive(owner); + process.nextTick(() => owner.emit('error', error)); } // Receives a chunk of data for a given stream and forwards it on // to the Http2Stream Duplex for processing. function onSessionRead(nread, buf, handle) { - const streams = this[kOwner][kState].streams; + const owner = this[kOwner]; + const streams = owner[kState].streams; const id = handle.id; const stream = streams.get(id); // It should not be possible for the stream to not exist at this point. @@ -303,7 +305,7 @@ function onSessionRead(nread, buf, handle) { 'Internal HTTP/2 Failure. Stream does not exist. Please ' + 'report this as a bug in Node.js'); const state = stream[kState]; - _unrefActive(this); // Reset the session timeout timer + _unrefActive(owner); // Reset the session timeout timer _unrefActive(stream); // Reset the stream timeout timer if (nread >= 0 && !stream.destroyed) { @@ -322,7 +324,7 @@ function onSessionRead(nread, buf, handle) { function onSettings(ack) { const owner = this[kOwner]; debug(`[${sessionName(owner[kType])}] new settings received`); - _unrefActive(this); + _unrefActive(owner); let event = 'remoteSettings'; if (ack) { if (owner[kState].pendingAck > 0) @@ -348,7 +350,7 @@ function onPriority(id, parent, weight, exclusive) { debug(`[${sessionName(owner[kType])}] priority advisement for stream ` + `${id}: \n parent: ${parent},\n weight: ${weight},\n` + ` exclusive: ${exclusive}`); - _unrefActive(this); + _unrefActive(owner); const streams = owner[kState].streams; const stream = streams.get(id); const emitter = stream === undefined ? owner : stream; @@ -370,7 +372,7 @@ function onFrameError(id, type, code) { const owner = this[kOwner]; debug(`[${sessionName(owner[kType])}] error sending frame type ` + `${type} on stream ${id}, code: ${code}`); - _unrefActive(this); + _unrefActive(owner); const streams = owner[kState].streams; const stream = streams.get(id); const emitter = stream !== undefined ? stream : owner; @@ -975,7 +977,7 @@ class Http2Session extends EventEmitter { state.destroying = true; // Unenroll the timer - this.setTimeout(0); + this.setTimeout(0, sessionOnTimeout); // Shut down any still open streams const streams = state.streams; @@ -2185,7 +2187,6 @@ function socketDestroy(error) { const type = this[kSession][kType]; debug(`[${sessionName(type)}] socket destroy called`); delete this[kServer]; - this.setTimeout(0); // destroy the session first so that it will stop trying to // send data while we close the socket. this[kSession].destroy(); @@ -2247,31 +2248,6 @@ function socketOnError(error) { this.destroy(error); } -// When the socket times out on the server, attempt a graceful shutdown -// of the session. -function socketOnTimeout() { - debug('socket timeout'); - process.nextTick(() => { - const server = this[kServer]; - const session = this[kSession]; - // If server or session are undefined, or session.destroyed is true - // then we're already in the process of shutting down, do nothing. - if (server === undefined || session === undefined) - return; - const state = session[kState]; - if (state.destroyed || state.destroying) - return; - if (!server.emit('timeout', session, this)) { - session.shutdown( - { - graceful: true, - errorCode: NGHTTP2_NO_ERROR - }, - this.destroy.bind(this)); - } - }); -} - // Handles the on('stream') event for a session and forwards // it on to the server object. function sessionOnStream(stream, headers, flags, rawHeaders) { @@ -2289,15 +2265,34 @@ function sessionOnSocketError(error, socket) { this[kServer].emit('socketError', error, socket, this); } +// When the session times out on the server, attempt a graceful shutdown +function sessionOnTimeout() { + debug('session timeout'); + process.nextTick(() => { + // if destroyed or destryoing, do nothing + if (this[kState].destroyed || this[kState].destroying) + return; + const server = this[kServer]; + const socket = this[kSocket]; + // If server or socket are undefined, then we're already in the process of + // shutting down, do nothing. + if (server === undefined || socket === undefined) + return; + if (!server.emit('timeout', this)) { + this.shutdown( + { + graceful: true, + errorCode: NGHTTP2_NO_ERROR + }, + socket.destroy.bind(socket)); + } + }); +} + function connectionListener(socket) { debug('[server] received a connection'); const options = this[kOptions] || {}; - if (this.timeout) { - socket.setTimeout(this.timeout); - socket.on('timeout', socketOnTimeout); - } - if (socket.alpnProtocol === false || socket.alpnProtocol === 'http/1.1') { if (options.allowHTTP1 === true) { // Fallback to HTTP/1.1 @@ -2325,6 +2320,11 @@ function connectionListener(socket) { session.on('priority', sessionOnPriority); session.on('socketError', sessionOnSocketError); + if (this.timeout) { + session.setTimeout(this.timeout); + session.on('timeout', sessionOnTimeout); + } + socket[kServer] = this; process.nextTick(emit.bind(this, 'session', session)); diff --git a/test/parallel/test-http2-session-timeout.js b/test/parallel/test-http2-session-timeout.js new file mode 100644 index 00000000000..27cab3d8bdb --- /dev/null +++ b/test/parallel/test-http2-session-timeout.js @@ -0,0 +1,41 @@ +// Flags: --expose-http2 +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const h2 = require('http2'); + +const serverTimeout = common.platformTimeout(200); +const callTimeout = common.platformTimeout(10); + +const server = h2.createServer(); +server.timeout = serverTimeout; + +server.on('request', (req, res) => res.end()); +server.on('timeout', common.mustNotCall()); + +server.listen(0, common.mustCall(() => { + const port = server.address().port; + + const url = `http://localhost:${port}`; + const client = h2.connect(url); + makeReq(40); + + function makeReq(attempts) { + const request = client.request({ + ':path': '/foobar', + ':method': 'GET', + ':scheme': 'http', + ':authority': `localhost:${port}`, + }); + request.end(); + + if (attempts) { + setTimeout(() => makeReq(attempts - 1), callTimeout); + } else { + server.close(); + client.destroy(); + } + } +})); |