http2: guard against destroyed session, timeouts

Guard against destroyed session in timeouts and goaway event.

Improve timeout handling a bit.

PR-URL: https://github.com/nodejs/node/pull/15106
Fixes: https://github.com/nodejs/node/issues/14964
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This commit is contained in:
James M Snell 2017-08-30 16:20:11 -07:00
parent ea2e6363f2
commit 09480a8bf0
3 changed files with 38 additions and 7 deletions

View File

@ -377,6 +377,8 @@ function onFrameError(id, type, code) {
function emitGoaway(state, code, lastStreamID, buf) { function emitGoaway(state, code, lastStreamID, buf) {
this.emit('goaway', code, lastStreamID, buf); this.emit('goaway', code, lastStreamID, buf);
// Tear down the session or destroy // Tear down the session or destroy
if (state.destroying || state.destroyed)
return;
if (!state.shuttingDown && !state.shutdown) { if (!state.shuttingDown && !state.shutdown) {
this.shutdown({}, this.destroy.bind(this)); this.shutdown({}, this.destroy.bind(this));
} else { } else {
@ -970,7 +972,7 @@ class Http2Session extends EventEmitter {
state.destroying = true; state.destroying = true;
// Unenroll the timer // Unenroll the timer
unenroll(this); this.setTimeout(0);
// Shut down any still open streams // Shut down any still open streams
const streams = state.streams; const streams = state.streams;
@ -1530,7 +1532,7 @@ class Http2Stream extends Duplex {
session.removeListener('close', this[kState].closeHandler); session.removeListener('close', this[kState].closeHandler);
// Unenroll the timer // Unenroll the timer
unenroll(this); this.setTimeout(0);
setImmediate(finishStreamDestroy.bind(this, handle)); setImmediate(finishStreamDestroy.bind(this, handle));
@ -2180,7 +2182,7 @@ function socketDestroy(error) {
const type = this[kSession][kType]; const type = this[kSession][kType];
debug(`[${sessionName(type)}] socket destroy called`); debug(`[${sessionName(type)}] socket destroy called`);
delete this[kServer]; delete this[kServer];
this.removeListener('timeout', socketOnTimeout); this.setTimeout(0);
// destroy the session first so that it will stop trying to // destroy the session first so that it will stop trying to
// send data while we close the socket. // send data while we close the socket.
this[kSession].destroy(); this[kSession].destroy();
@ -2249,10 +2251,13 @@ function socketOnTimeout() {
process.nextTick(() => { process.nextTick(() => {
const server = this[kServer]; const server = this[kServer];
const session = this[kSession]; const session = this[kSession];
// If server or session are undefined, then we're already in the process of // If server or session are undefined, or session.destroyed is true
// shutting down, do nothing. // then we're already in the process of shutting down, do nothing.
if (server === undefined || session === undefined) if (server === undefined || session === undefined)
return; return;
const state = session[kState];
if (state.destroyed || state.destroying)
return;
if (!server.emit('timeout', session, this)) { if (!server.emit('timeout', session, this)) {
session.shutdown( session.shutdown(
{ {

View File

@ -0,0 +1,25 @@
// Flags: --expose-http2
'use strict';
const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const http2 = require('http2');
const server = http2.createServer();
server.on('stream', common.mustCall((stream) => {
stream.on('error', common.mustCall());
stream.session.shutdown();
}));
server.listen(0, common.mustCall(() => {
const client = http2.connect(`http://localhost:${server.address().port}`);
client.on('goaway', common.mustCall(() => {
// We ought to be able to destroy the client in here without an error
server.close();
client.destroy();
}));
client.request();
}));

View File

@ -26,9 +26,10 @@ server.on('listening', common.mustCall(() => {
const client = h2.connect(`http://localhost:${server.address().port}`); const client = h2.connect(`http://localhost:${server.address().port}`);
const req = client.request({ ':path': '/' }); client.on('goaway', common.mustCall());
const req = client.request();
req.resume(); req.resume();
req.on('end', common.mustCall(() => server.close())); req.on('end', common.mustCall(() => server.close()));
req.end();
})); }));