http2: avoid bind and properly clean up in compat

PR-URL: nodejs#20374 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
kjin · Sep 10, 2018 · fce762e · fce762e
1 parent 2dec831
commit fce762e
Showing 1 changed file with 46 additions and 32 deletions.
diff --git 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];