From e0a9ad1af244f8756a228a6d087b3a55ee4c0d14 Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Sun, 19 Mar 2017 20:58:31 +0100 Subject: [PATCH] http: avoid retaining unneeded memory Prevent the events listeners of the sockets obtained with the HTTP upgrade mechanism from retaining unneeded memory. Ref: https://github.com/nodejs/node/issues/11868 PR-URL: https://github.com/nodejs/node/pull/11926 Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Ben Noordhuis --- lib/_http_common.js | 6 +----- lib/_http_server.js | 43 ++++++++++++++++++++++--------------------- lib/internal/http.js | 7 ++++++- 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/lib/_http_common.js b/lib/_http_common.js index 2245ae079acedd..52cabd87fdfcab 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -26,6 +26,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; @@ -222,11 +223,6 @@ function freeParser(parser, req, socket) { } -function ondrain() { - if (this._httpMessage) this._httpMessage.emit('drain'); -} - - function httpSocketSetup(socket) { socket.removeListener('drain', ondrain); socket.on('drain', ondrain); diff --git a/lib/_http_server.js b/lib/_http_server.js index a95fe1d21cc8ea..7b3f004f9aff03 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -34,7 +34,7 @@ const continueExpression = common.continueExpression; const chunkExpression = common.chunkExpression; const httpSocketSetup = common.httpSocketSetup; const OutgoingMessage = require('_http_outgoing').OutgoingMessage; -const outHeadersKey = require('internal/http').outHeadersKey; +const { outHeadersKey, ondrain } = require('internal/http'); const STATUS_CODES = { 100: 'Continue', @@ -296,7 +296,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); @@ -314,9 +314,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 @@ -326,21 +326,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; @@ -378,15 +377,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) { @@ -413,7 +412,7 @@ function socketOnEnd(server, socket, parser, state) { if (ret instanceof Error) { debug('parse error'); - state.onError(ret); + socketOnError.call(socket, ret); return; } @@ -443,19 +442,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; @@ -468,6 +467,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); diff --git a/lib/internal/http.js b/lib/internal/http.js index 1ba68a41a5692b..0b12bc7c8fbe85 100644 --- a/lib/internal/http.js +++ b/lib/internal/http.js @@ -1,5 +1,10 @@ 'use strict'; +function ondrain() { + if (this._httpMessage) this._httpMessage.emit('drain'); +} + module.exports = { - outHeadersKey: Symbol('outHeadersKey') + outHeadersKey: Symbol('outHeadersKey'), + ondrain, };