From 2fc528ce009a455a69f5984e62c881a05cd4ca37 Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 4 May 2012 10:40:27 -0700 Subject: [PATCH] http: Clean up parser usage Move parsers.free(parser) to a single function, which also nulls all of the various references we hang on them. Also, move the parser.on* methods out of the closure, so that there's one shared definition of each, instead of re-defining for each parser in a spot where they can close over references to other request-specific objects. --- lib/http.js | 239 ++++++++++++++++++++++++++++------------------------ 1 file changed, 128 insertions(+), 111 deletions(-) diff --git a/lib/http.js b/lib/http.js index 4c55db0c8c..ef42584322 100644 --- a/lib/http.js +++ b/lib/http.js @@ -35,113 +35,120 @@ if (process.env.NODE_DEBUG && /http/.test(process.env.NODE_DEBUG)) { debug = function() { }; } +function parserOnHeaders(headers, url) { + this._headers = this._headers.concat(headers); + this._url += url; +} -var parsers = new FreeList('parsers', 1000, function() { - var parser = new HTTPParser(HTTPParser.REQUEST); +// info.headers and info.url are set only if .onHeaders() +// has not been called for this request. +// +// info.url is not set for response parsers but that's not +// applicable here since all our parsers are request parsers. +function parserOnHeadersComplete(info) { + var parser = this; + var headers = info.headers; + var url = info.url; + + if (!headers) { + headers = parser._headers; + parser._headers = []; + } - parser._headers = []; - parser._url = ''; + if (!url) { + url = parser._url; + parser._url = ''; + } - // Only called in the slow case where slow means - // that the request headers were either fragmented - // across multiple TCP packets or too large to be - // processed in a single run. This method is also - // called to process trailing HTTP headers. - parser.onHeaders = function(headers, url) { - parser._headers = parser._headers.concat(headers); - parser._url += url; - }; + parser.incoming = new IncomingMessage(parser.socket); + parser.incoming.httpVersionMajor = info.versionMajor; + parser.incoming.httpVersionMinor = info.versionMinor; + parser.incoming.httpVersion = info.versionMajor + '.' + info.versionMinor; + parser.incoming.url = url; - // info.headers and info.url are set only if .onHeaders() - // has not been called for this request. - // - // info.url is not set for response parsers but that's not - // applicable here since all our parsers are request parsers. - parser.onHeadersComplete = function(info) { - var headers = info.headers; - var url = info.url; - - if (!headers) { - headers = parser._headers; - parser._headers = []; - } + for (var i = 0, n = headers.length; i < n; i += 2) { + var k = headers[i]; + var v = headers[i + 1]; + parser.incoming._addHeaderLine(k.toLowerCase(), v); + } - if (!url) { - url = parser._url; - parser._url = ''; - } + if (info.method) { + // server only + parser.incoming.method = info.method; + } else { + // client only + parser.incoming.statusCode = info.statusCode; + // CHECKME dead code? we're always a request parser + } - parser.incoming = new IncomingMessage(parser.socket); - parser.incoming.httpVersionMajor = info.versionMajor; - parser.incoming.httpVersionMinor = info.versionMinor; - parser.incoming.httpVersion = info.versionMajor + '.' + info.versionMinor; - parser.incoming.url = url; + parser.incoming.upgrade = info.upgrade; - for (var i = 0, n = headers.length; i < n; i += 2) { - var k = headers[i]; - var v = headers[i + 1]; - parser.incoming._addHeaderLine(k.toLowerCase(), v); - } + var isHeadResponse = false; - if (info.method) { - // server only - parser.incoming.method = info.method; - } else { - // client only - parser.incoming.statusCode = info.statusCode; - // CHECKME dead code? we're always a request parser - } + if (!info.upgrade) { + // For upgraded connections, we'll emit this after parser.execute + // so that we can capture the first part of the new protocol + isHeadResponse = parser.onIncoming(parser.incoming, info.shouldKeepAlive); + } - parser.incoming.upgrade = info.upgrade; + return isHeadResponse; +} - var isHeadResponse = false; +function parserOnBody(b, start, len) { + var parser = this; + // TODO body encoding? + var slice = b.slice(start, start + len); + if (parser.incoming._decoder) { + var string = parser.incoming._decoder.write(slice); + if (string.length) parser.incoming.emit('data', string); + } else { + parser.incoming.emit('data', slice); + } +} +function parserOnMessageComplete() { + var parser = this; + parser.incoming.complete = true; - if (!info.upgrade) { - // For upgraded connections, we'll emit this after parser.execute - // so that we can capture the first part of the new protocol - isHeadResponse = parser.onIncoming(parser.incoming, info.shouldKeepAlive); + // Emit any trailing headers. + var headers = parser._headers; + if (headers) { + for (var i = 0, n = headers.length; i < n; i += 2) { + var k = headers[i]; + var v = headers[i + 1]; + parser.incoming._addHeaderLine(k.toLowerCase(), v); } + parser._headers = []; + parser._url = ''; + } - return isHeadResponse; - }; + if (!parser.incoming.upgrade) { + // For upgraded connections, also emit this after parser.execute + parser.incoming.readable = false; + parser.incoming.emit('end'); + } - parser.onBody = function(b, start, len) { - // TODO body encoding? - var slice = b.slice(start, start + len); - if (parser.incoming._decoder) { - var string = parser.incoming._decoder.write(slice); - if (string.length) parser.incoming.emit('data', string); - } else { - parser.incoming.emit('data', slice); - } - }; + if (parser.socket.readable) { + // force to read the next incoming message + parser.socket.resume(); + } +} - parser.onMessageComplete = function() { - parser.incoming.complete = true; - // Emit any trailing headers. - var headers = parser._headers; - if (headers) { - for (var i = 0, n = headers.length; i < n; i += 2) { - var k = headers[i]; - var v = headers[i + 1]; - parser.incoming._addHeaderLine(k.toLowerCase(), v); - } - parser._headers = []; - parser._url = ''; - } +var parsers = new FreeList('parsers', 1000, function() { + var parser = new HTTPParser(HTTPParser.REQUEST); - if (!parser.incoming.upgrade) { - // For upgraded connections, also emit this after parser.execute - parser.incoming.readable = false; - parser.incoming.emit('end'); - } + parser._headers = []; + parser._url = ''; - if (parser.socket.readable) { - // force to read the next incoming message - parser.socket.resume(); - } - }; + // Only called in the slow case where slow means + // that the request headers were either fragmented + // across multiple TCP packets or too large to be + // processed in a single run. This method is also + // called to process trailing HTTP headers. + parser.onHeaders = parserOnHeaders; + parser.onHeadersComplete = parserOnHeadersComplete; + parser.onBody = parserOnBody; + parser.onMessageComplete = parserOnMessageComplete; return parser; }); @@ -1110,6 +1117,31 @@ function createHangUpError() { return error; } +// Free the parser and also break any links that it +// might have to any other things. +// TODO: All parser data should be attached to a +// single object, so that it can be easily cleaned +// up by doing `parser.data = {}`, which should +// be done in FreeList.free. `parsers.free(parser)` +// should be all that is needed. +function freeParser(parser, req) { + if (parser) { + parser._headers = []; + parser.onIncoming = null; + if (parser.socket) { + parser.socket.onend = null; + parser.socket.ondata = null; + } + parser.socket = null; + parser.incoming = null; + parsers.free(parser); + parser = null; + } + if (req) { + req.parser = null; + } +}; + ClientRequest.prototype.onSocket = function(socket) { var req = this; @@ -1126,21 +1158,6 @@ ClientRequest.prototype.onSocket = function(socket) { // Setup "drain" propogation. httpSocketSetup(socket); - var freeParser = function() { - if (parser) { - parser.onIncoming = null; - parser.socket.onend = null; - parser.socket.ondata = null; - parser.socket = null; - parser.incoming = null; - parsers.free(parser); - parser = null; - } - if (req) { - req.parser = null; - } - }; - var errorListener = function(err) { debug('HTTP SOCKET ERROR: ' + err.message + '\n' + err.stack); req.emit('error', err); @@ -1149,7 +1166,7 @@ ClientRequest.prototype.onSocket = function(socket) { req._hadError = true; if (parser) { parser.finish(); - freeParser(); + freeParser(parser, req); } socket.destroy(); } @@ -1159,7 +1176,7 @@ ClientRequest.prototype.onSocket = function(socket) { var ret = parser.execute(d, start, end - start); if (ret instanceof Error) { debug('parse error'); - freeParser(); + freeParser(parser, req); socket.destroy(ret); } else if (parser.incoming && parser.incoming.upgrade) { var bytesParsed = ret; @@ -1180,13 +1197,13 @@ ClientRequest.prototype.onSocket = function(socket) { // Got upgrade header, but have no handler. socket.destroy(); } - freeParser(); + freeParser(parser, req); } else if (parser.incoming && parser.incoming.complete && // When the status code is 100 (Continue), the server will // send a final response after this client sends a request // body. So, we must not free the parser. parser.incoming.statusCode !== 100) { - freeParser(); + freeParser(parser, req); } }; @@ -1199,7 +1216,7 @@ ClientRequest.prototype.onSocket = function(socket) { } if (parser) { parser.finish(); - freeParser(); + freeParser(parser, req); } socket.destroy(); }; @@ -1494,8 +1511,8 @@ function connectionListener(socket) { socket.addListener('close', function() { debug('server socket close'); - // unref the parser for easy gc - parsers.free(parser); + // mark this parser as reusable + freeParser(parser); abortIncoming(); });