Skip to content
Browse files

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.
  • Loading branch information...
1 parent 62c12d2 commit 2fc528ce009a455a69f5984e62c881a05cd4ca37 @isaacs committed
Showing with 128 additions and 111 deletions.
  1. +128 −111 lib/http.js
View
239 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();
});

0 comments on commit 2fc528c

Please sign in to comment.
Something went wrong with that request. Please try again.