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.

Conflicts:

	lib/http.js

1 files changed, 143 insertions, 123 deletions

diff --git a/lib/http.js b/lib/http.js
index 2468e2c84c..5216abed2c 100644
--- a/lib/http.js
+++ b/lib/http.js
@@ -36,128 +36,138 @@ if (process.env.NODE_DEBUG && /http/.test(process.env.NODE_DEBUG)) {
   debug = function() { };
 }
 
+// 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.
+function parserOnHeaders(headers, url) {
+  // Once we exceeded headers limit - stop collecting them
+  if (this.maxHeaderPairs <= 0 ||
+      this._headers.length < this.maxHeaderPairs) {
+    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;
 
-  parser._headers = [];
-  parser._url = '';
+  if (!headers) {
+    headers = parser._headers;
+    parser._headers = [];
+  }
 
-  // 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) {
-    // Once we exceeded headers limit - stop collecting them
-    if (parser.maxHeaderPairs <= 0 ||
-        parser._headers.length < parser.maxHeaderPairs) {
-      parser._headers = parser._headers.concat(headers);
-    }
-    parser._url += url;
-  };
+  if (!url) {
+    url = parser._url;
+    parser._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 = [];
-    }
+  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;
 
-    if (!url) {
-      url = parser._url;
-      parser._url = '';
-    }
+  var n = headers.length;
 
-    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;
+  // If parser.maxHeaderPairs <= 0 - assume that there're no limit
+  if (parser.maxHeaderPairs > 0) {
+    n = Math.min(n, parser.maxHeaderPairs);
+  }
 
-    var n = headers.length;
+  for (var i = 0; i < n; i += 2) {
+    var k = headers[i];
+    var v = headers[i + 1];
+    parser.incoming._addHeaderLine(k.toLowerCase(), v);
+  }
 
-    // If parser.maxHeaderPairs <= 0 - assume that there're no limit
-    if (parser.maxHeaderPairs > 0) {
-      n = Math.min(n, parser.maxHeaderPairs);
-    }
 
-    for (var i = 0; i < n; i += 2) {
-      var k = headers[i];
-      var v = headers[i + 1];
-      parser.incoming._addHeaderLine(k.toLowerCase(), v);
-    }
+  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.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.upgrade = info.upgrade;
 
-    parser.incoming.upgrade = info.upgrade;
+  var skipBody = false; // response to HEAD or CONNECT
 
-    var skipBody = false; // response to HEAD or CONNECT
+  if (!info.upgrade) {
+    // For upgraded connections and CONNECT method request,
+    // we'll emit this after parser.execute
+    // so that we can capture the first part of the new protocol
+    skipBody = parser.onIncoming(parser.incoming, info.shouldKeepAlive);
+  }
 
-    if (!info.upgrade) {
-      // For upgraded connections and CONNECT method request,
-      // we'll emit this after parser.execute
-      // so that we can capture the first part of the new protocol
-      skipBody = parser.onIncoming(parser.incoming, info.shouldKeepAlive);
-    }
+  return skipBody;
+}
 
-    return skipBody;
-  };
+function parserOnBody(b, start, len) {
+  var parser = this;
+  var slice = b.slice(start, start + len);
+  // body encoding is done in _emitData
+  parser.incoming._emitData(slice);
+}
 
-  parser.onBody = function(b, start, len) {
-    // TODO body encoding?
-    var slice = b.slice(start, start + len);
+function parserOnMessageComplete() {
+  var parser = this;
+  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 = '';
+  }
+
+  if (!parser.incoming.upgrade) {
+    // For upgraded connections, also emit this after parser.execute
     if (parser.incoming._paused || parser.incoming._pendings.length) {
-      parser.incoming._pendings.push(slice);
+      parser.incoming._pendings.push(END_OF_FILE);
     } else {
-      parser.incoming._emitData(slice);
+      parser.incoming.readable = false;
+      parser.incoming._emitEnd();
     }
-  };
+  }
 
-  parser.onMessageComplete = function() {
-    parser.incoming.complete = true;
+  if (parser.socket.readable) {
+    // force to read the next incoming message
+    parser.socket.resume();
+  }
+}
 
-    // 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 = '';
-    }
 
-    if (!parser.incoming.upgrade) {
-      // For upgraded connections, also emit this after parser.execute
-      if (parser.incoming._paused || parser.incoming._pendings.length) {
-        parser.incoming._pendings.push(END_OF_FILE);
-      } else {
-        parser.incoming.readable = false;
-        parser.incoming._emitEnd();
-      }
-    }
+var parsers = new FreeList('parsers', 1000, function() {
+  var parser = new HTTPParser(HTTPParser.REQUEST);
 
-    if (parser.socket.readable) {
-      // force to read the next incoming message
-      parser.socket.resume();
-    }
-  };
+  parser._headers = [];
+  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 = parserOnHeaders;
+  parser.onHeadersComplete = parserOnHeadersComplete;
+  parser.onBody = parserOnBody;
+  parser.onMessageComplete = parserOnMessageComplete;
 
   return parser;
 });
@@ -1230,6 +1240,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;
@@ -1254,21 +1289,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);
@@ -1277,7 +1297,7 @@ ClientRequest.prototype.onSocket = function(socket) {
       req._hadError = true;
       if (parser) {
         parser.finish();
-        freeParser();
+        freeParser(parser, req);
       }
       socket.destroy();
     }
@@ -1299,7 +1319,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) {
         // Upgrade or CONNECT
@@ -1310,7 +1330,6 @@ ClientRequest.prototype.onSocket = function(socket) {
         socket.ondata = null;
         socket.onend = null;
         parser.finish();
-        freeParser();
 
         // This is start + byteParsed
         var bodyHead = d.slice(start + bytesParsed, end);
@@ -1331,12 +1350,13 @@ ClientRequest.prototype.onSocket = function(socket) {
           // Got Upgrade header or CONNECT method, but have no handler.
           socket.destroy();
         }
+        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);
       }
     };
 
@@ -1349,7 +1369,7 @@ ClientRequest.prototype.onSocket = function(socket) {
       }
       if (parser) {
         parser.finish();
-        freeParser();
+        freeParser(parser, req);
       }
       socket.destroy();
     };
@@ -1586,8 +1606,8 @@ function connectionListener(socket) {
 
   function serverSocketCloseListener() {
     debug('server socket close');
-    // unref the parser for easy gc
-    parsers.free(parser);
+    // mark this parser as reusable
+    freeParser(parser);
 
     abortIncoming();
   }
@@ -1632,7 +1652,7 @@ function connectionListener(socket) {
       socket.onend = null;
       socket.removeListener('close', serverSocketCloseListener);
       parser.finish();
-      parsers.free(parser);
+      freeParser(parser, req);
 
       // This is start + byteParsed
       var bodyHead = d.slice(start + bytesParsed, end);