http: avoid retaining unneeded memory

Prevent the events listeners of the sockets obtained with the HTTP
upgrade mechanism from retaining unneeded memory.

Ref: nodejs#11868
PR-URL: nodejs#11926
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>

Author: lpinca

lib/_http_common.js

@@ -5,6 +5,7 @@ const methods = binding.methods;
 const HTTPParser = binding.HTTPParser;
 
 const FreeList = require('internal/freelist').FreeList;
+const ondrain = require('internal/http').ondrain;
 const incoming = require('_http_incoming');
 const IncomingMessage = incoming.IncomingMessage;
 const readStart = incoming.readStart;
@@ -209,11 +210,6 @@ function freeParser(parser, req, socket) {
 exports.freeParser = freeParser;
 
 
-function ondrain() {
-  if (this._httpMessage) this._httpMessage.emit('drain');
-}
-
-
 function httpSocketSetup(socket) {
   socket.removeListener('drain', ondrain);
   socket.on('drain', ondrain);

lib/_http_server.js

@@ -13,6 +13,7 @@ const continueExpression = common.continueExpression;
 const chunkExpression = common.chunkExpression;
 const httpSocketSetup = common.httpSocketSetup;
 const OutgoingMessage = require('_http_outgoing').OutgoingMessage;
+const ondrain = require('internal/http').ondrain;
 
 const STATUS_CODES = exports.STATUS_CODES = {
   100: 'Continue',
@@ -274,7 +275,7 @@ function connectionListener(socket) {
   // otherwise, destroy on timeout by default
   if (this.timeout)
     socket.setTimeout(this.timeout);
-  socket.on('timeout', socketOnTimeout.bind(undefined, this, socket));
+  socket.on('timeout', socketOnTimeout);
 
   var parser = parsers.alloc();
   parser.reinitialize(HTTPParser.REQUEST);
@@ -292,9 +293,9 @@ function connectionListener(socket) {
 
   var state = {
     onData: null,
-    onError: null,
     onEnd: null,
     onClose: null,
+    onDrain: null,
     outgoing: [],
     incoming: [],
     // `outgoingData` is an approximate amount of bytes queued through all
@@ -304,21 +305,20 @@ function connectionListener(socket) {
     outgoingData: 0
   };
   state.onData = socketOnData.bind(undefined, this, socket, parser, state);
-  state.onError = socketOnError.bind(undefined, this, socket, state);
   state.onEnd = socketOnEnd.bind(undefined, this, socket, parser, state);
   state.onClose = socketOnClose.bind(undefined, socket, state);
+  state.onDrain = socketOnDrain.bind(undefined, socket, state);
   socket.on('data', state.onData);
-  socket.on('error', state.onError);
+  socket.on('error', socketOnError);
   socket.on('end', state.onEnd);
   socket.on('close', state.onClose);
+  socket.on('drain', state.onDrain);
   parser.onIncoming = parserOnIncoming.bind(undefined, this, socket, state);
 
   // We are consuming socket, so it won't get any actual data
   socket.on('resume', onSocketResume);
   socket.on('pause', onSocketPause);
 
-  socket.on('drain', socketOnDrain.bind(undefined, socket, state));
-
   // Override on to unconsume on `data`, `readable` listeners
   socket.on = socketOnWrap;
 
@@ -356,15 +356,15 @@ function socketOnDrain(socket, state) {
   }
 }
 
-function socketOnTimeout(server, socket) {
-  var req = socket.parser && socket.parser.incoming;
-  var reqTimeout = req && !req.complete && req.emit('timeout', socket);
-  var res = socket._httpMessage;
-  var resTimeout = res && res.emit('timeout', socket);
-  var serverTimeout = server.emit('timeout', socket);
+function socketOnTimeout() {
+  var req = this.parser && this.parser.incoming;
+  var reqTimeout = req && !req.complete && req.emit('timeout', this);
+  var res = this._httpMessage;
+  var resTimeout = res && res.emit('timeout', this);
+  var serverTimeout = this.server.emit('timeout', this);
 
   if (!reqTimeout && !resTimeout && !serverTimeout)
-    socket.destroy();
+    this.destroy();
 }
 
 function socketOnClose(socket, state) {
@@ -391,7 +391,7 @@ function socketOnEnd(server, socket, parser, state) {
 
   if (ret instanceof Error) {
     debug('parse error');
-    state.onError(ret);
+    socketOnError.call(socket, ret);
     return;
   }
 
@@ -421,19 +421,19 @@ function onParserExecute(server, socket, parser, state, ret, d) {
   onParserExecuteCommon(server, socket, parser, state, ret, undefined);
 }
 
-function socketOnError(server, socket, state, e) {
+function socketOnError(e) {
   // Ignore further errors
-  socket.removeListener('error', state.onError);
-  socket.on('error', () => {});
+  this.removeListener('error', socketOnError);
+  this.on('error', () => {});
 
-  if (!server.emit('clientError', e, socket))
-    socket.destroy(e);
+  if (!this.server.emit('clientError', e, this))
+    this.destroy(e);
 }
 
 function onParserExecuteCommon(server, socket, parser, state, ret, d) {
   if (ret instanceof Error) {
     debug('parse error');
-    state.onError(ret);
+    socketOnError.call(socket, ret);
   } else if (parser.incoming && parser.incoming.upgrade) {
     // Upgrade or CONNECT
     var bytesParsed = ret;
@@ -446,6 +446,8 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
     socket.removeListener('data', state.onData);
     socket.removeListener('end', state.onEnd);
     socket.removeListener('close', state.onClose);
+    socket.removeListener('drain', state.onDrain);
+    socket.removeListener('drain', ondrain);
     unconsume(parser, socket);
     parser.finish();
     freeParser(parser, req, null);

lib/internal/http.js

@@ -0,0 +1,9 @@
+'use strict';
+
+function ondrain() {
+  if (this._httpMessage) this._httpMessage.emit('drain');
+}
+
+module.exports = {
+  ondrain,
+};

node.gyp

@@ -85,6 +85,7 @@
       'lib/internal/errors.js',
       'lib/internal/freelist.js',
       'lib/internal/fs.js',
+      'lib/internal/http.js',
       'lib/internal/linkedlist.js',
       'lib/internal/net.js',
       'lib/internal/module.js',