http: reduce creating http-parser instances

Fixes #2577.
nodejs · Jan 21, 2012 · 35fe3eb · 35fe3eb
1 parent 40c9348
commit 35fe3eb
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 3 deletions.
diff --git a/lib/http.js b/lib/http.js
@@ -1122,13 +1122,23 @@ ClientRequest.prototype.onSocket = function(socket) {
     // Setup "drain" propogation.
     httpSocketSetup(socket);
 
+    var freeParser = function() {
+      if (parser) {
+        parsers.free(parser);
+        parser = null;
+      }
+    };
+
     var errorListener = function(err) {
       debug('HTTP SOCKET ERROR: ' + err.message + '\n' + err.stack);
       req.emit('error', err);
       // For Safety. Some additional errors might fire later on
       // and we need to make sure we don't double-fire the error event.
       req._hadError = true;
-      parser.finish();
+      if (parser) {
+        parser.finish();
+        freeParser();
+      }
       socket.destroy();
     }
     socket.on('error', errorListener);
@@ -1137,6 +1147,7 @@ ClientRequest.prototype.onSocket = function(socket) {
       var ret = parser.execute(d, start, end - start);
       if (ret instanceof Error) {
         debug('parse error');
+        freeParser();
         socket.destroy(ret);
       } else if (parser.incoming && parser.incoming.upgrade) {
         var bytesParsed = ret;
@@ -1157,6 +1168,9 @@ ClientRequest.prototype.onSocket = function(socket) {
           // Got upgrade header, but have no handler.
           socket.destroy();
         }
+        freeParser();
+      } else if (parser.incoming && parser.incoming.complete) {
+        freeParser();
       }
     };
 
@@ -1167,8 +1181,10 @@ ClientRequest.prototype.onSocket = function(socket) {
         req.emit('error', createHangUpError());
         req._hadError = true;
       }
-      parser.finish();
-      parsers.free(parser); // I don't know if this is necessary --Mikeal
+      if (parser) {
+        parser.finish();
+        freeParser();
+      }
       socket.destroy();
     };
 

diff --git a/test/simple/test-http-parser-free.js b/test/simple/test-http-parser-free.js
@@ -0,0 +1,54 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+var http = require('http');
+var N = 100;
+var responses = 0;
+
+var server = http.createServer(function(req, res) {
+  res.end('Hello');
+});
+
+server.listen(common.PORT, function() {
+  http.globalAgent.maxSockets = 1;
+  var parser;
+  for (var i = 0; i < N; ++i) {
+    (function makeRequest(i) {
+      var req = http.get({port: common.PORT}, function(res) {
+        if (!parser) {
+          parser = req.parser;
+        } else {
+          assert.strictEqual(req.parser, parser);
+        }
+
+        if (++responses === N) {
+          server.close();
+        }
+      });
+    })(i);
+  }
+});
+
+process.on('exit', function() {
+  assert.equal(responses, N);
+});