From fce762e298c01ff079d7b4765182ea2dfa77d1cf Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 28 Apr 2018 00:34:52 +0200 Subject: [PATCH] http2: avoid bind and properly clean up in compat PR-URL: https://github.com/nodejs/node/pull/20374 Reviewed-By: Anatoli Papirovski Reviewed-By: James M Snell Reviewed-By: Trivikram Kamat --- lib/internal/http2/compat.js | 78 +++++++++++++++++++++--------------- 1 file changed, 46 insertions(+), 32 deletions(-) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index 02b595b6a023f8..009bc7a2ac7ab6 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -7,7 +7,6 @@ const constants = binding.constants; const errors = require('internal/errors'); const { kSocket } = require('internal/http2/util'); -const kFinish = Symbol('finish'); const kBeginSend = Symbol('begin-send'); const kState = Symbol('state'); const kStream = Symbol('stream'); @@ -211,6 +210,27 @@ const proxySocketHandler = { } }; +function onStreamCloseRequest() { + const req = this[kRequest]; + + if (req === undefined) + return; + + const state = req[kState]; + state.closed = true; + + req.push(null); + // if the user didn't interact with incoming data and didn't pipe it, + // dump it for compatibility with http1 + if (!state.didRead && !req._readableState.resumeScheduled) + req.resume(); + + this[kProxySocket] = null; + this[kRequest] = undefined; + + req.emit('close'); +} + class Http2ServerRequest extends Readable { constructor(stream, headers, options, rawHeaders) { super(options); @@ -234,7 +254,7 @@ class Http2ServerRequest extends Readable { stream.on('end', onStreamEnd); stream.on('error', onStreamError); stream.on('aborted', onStreamAbortedRequest); - stream.on('close', this[kFinish].bind(this)); + stream.on('close', onStreamCloseRequest); this.on('pause', onRequestPause); this.on('resume', onRequestResume); } @@ -335,24 +355,30 @@ class Http2ServerRequest extends Readable { return; this[kStream].setTimeout(msecs, callback); } - - [kFinish]() { - const state = this[kState]; - if (state.closed) - return; - state.closed = true; - this.push(null); - this[kStream][kRequest] = undefined; - // if the user didn't interact with incoming data and didn't pipe it, - // dump it for compatibility with http1 - if (!state.didRead && !this._readableState.resumeScheduled) - this.resume(); - this.emit('close'); - } } function onStreamTrailersReady() { - this[kStream].sendTrailers(this[kTrailers]); + this.sendTrailers(this[kResponse][kTrailers]); +} + +function onStreamCloseResponse() { + const res = this[kResponse]; + + if (res === undefined) + return; + + const state = res[kState]; + + if (this.headRequest !== state.headRequest) + return; + + state.closed = true; + + this[kProxySocket] = null; + this[kResponse] = undefined; + + res.emit('finish'); + res.emit('close'); } class Http2ServerResponse extends Stream { @@ -373,8 +399,8 @@ class Http2ServerResponse extends Stream { this.writable = true; stream.on('drain', onStreamDrain); stream.on('aborted', onStreamAbortedResponse); - stream.on('close', this[kFinish].bind(this)); - stream.on('wantTrailers', onStreamTrailersReady.bind(this)); + stream.on('close', onStreamCloseResponse); + stream.on('wantTrailers', onStreamTrailersReady); } // User land modules such as finalhandler just check truthiness of this @@ -605,7 +631,7 @@ class Http2ServerResponse extends Stream { this.writeHead(this[kState].statusCode); if (isFinished) - this[kFinish](); + onStreamCloseResponse.call(stream); else stream.end(); } @@ -649,18 +675,6 @@ class Http2ServerResponse extends Stream { this[kStream].respond(headers, options); } - [kFinish]() { - const stream = this[kStream]; - const state = this[kState]; - if (state.closed || stream.headRequest !== state.headRequest) - return; - state.closed = true; - this[kProxySocket] = null; - stream[kResponse] = undefined; - this.emit('finish'); - this.emit('close'); - } - // TODO doesn't support callbacks writeContinue() { const stream = this[kStream];