Merge pull request #11 from retool/0.2.x

not identifying destroyed state on sockets when SSL - node 0.10.32
node-modules · Oct 16, 2014 · efab212 · efab212
2 parents e3a255e + d5df4b1
commit efab212
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 14 deletions.
diff --git a/lib/_http_agent.js b/lib/_http_agent.js
@@ -76,7 +76,7 @@ function Agent(options) {
     var name = self.getName(options);
     debug('agent.on(free)', name);
 
-    if (!socket.destroyed &&
+    if (!self.isDestroyed(socket) &&
         self.requests[name] && self.requests[name].length) {
       self.requests[name].shift().onSocket(socket);
       if (self.requests[name].length === 0) {
@@ -89,7 +89,7 @@ function Agent(options) {
       var req = socket._httpMessage;
       if (req &&
           req.shouldKeepAlive &&
-          !socket.destroyed &&
+          !self.isDestroyed(socket) &&
           self.options.keepAlive) {
         var freeSockets = self.freeSockets[name];
         var freeLen = freeSockets ? freeSockets.length : 0;
@@ -240,9 +240,9 @@ Agent.prototype.createSocket = function(req, options) {
 
 Agent.prototype.removeSocket = function(s, options) {
   var name = this.getName(options);
-  debug('removeSocket', name, 'destroyed:', s.destroyed);
+  debug('removeSocket', name, 'destroyed:', this.isDestroyed(s));
   var sets = [this.sockets];
-  if (s.destroyed) {
+  if (this.isDestroyed(s)) {
     // If the socket was destroyed, we need to remove it from the free buffers.
     sets.push(this.freeSockets);
   }
@@ -320,4 +320,7 @@ Agent.prototype.get = function(options, cb) {
   return req;
 };
 
+Agent.prototype.isDestroyed = function(socket){
+  return socket.destroyed || socket._destroyed;
+}
 exports.globalAgent = new Agent();
diff --git a/lib/agent.js b/lib/agent.js
@@ -60,7 +60,7 @@ function Agent(options) {
     var name = self.getName(options);
     debug('agent.on(free)', name);
 
-    if (!socket.destroyed &&
+    if (!self.isDestroyed(socket) &&
         self.requests[name] && self.requests[name].length) {
       self.requests[name].shift().onSocket(socket);
       if (self.requests[name].length === 0) {
@@ -73,7 +73,7 @@ function Agent(options) {
       var req = socket._httpMessage;
       if (req &&
           req.shouldKeepAlive &&
-          !socket.destroyed &&
+          !self.isDestroyed(socket) &&
           self.options.keepAlive) {
         var freeSockets = self.freeSockets[name];
         var freeLen = freeSockets ? freeSockets.length : 0;
@@ -139,9 +139,9 @@ Agent.prototype.createSocket = function (req, options) {
 Agent.prototype.removeSocket = function (s, options) {
   OriginalAgent.prototype.removeSocket.call(this, s, options);
   var name = this.getName(options);
-  debug('removeSocket', name, 'destroyed:', s.destroyed);
+  debug('removeSocket', name, 'destroyed:', this.isDestroyed(s));
 
-  if (s.destroyed && this.freeSockets[name]) {
+  if (this.isDestroyed(s) && this.freeSockets[name]) {
     var index = this.freeSockets[name].indexOf(s);
     if (index !== -1) {
       this.freeSockets[name].splice(index, 1);

diff --git a/test/https_agent.test.js b/test/https_agent.test.js
@@ -29,20 +29,20 @@ describe.only('https_agent.test.js', function () {
 
   it('should GET / success with 200 status', function (done) {
     agentkeepalive.get({
-      hostname: 'npm.taobao.org',
+      hostname: "www.google.com",
       port: 443,
-      path: '/',
+      path: '/search?q=nodejs',
     }, function (res) {
       res.statusCode.should.equal(200);
       done();
     });
   });
 
-  it('should GET / and /package/agentkeepalive use the same socket', function (done) {
+  it('should GET /search?q=nodejs /search?q=agentkeepalive use the same socket', function (done) {
     var options = {
-      hostname: 'npm.taobao.org',
+      hostname: 'www.google.com',
       port: 443,
-      path: '/',
+      path: '/search?q=nodejs',
       agent: agentkeepalive,
     };
     var remotePort = null;
@@ -55,7 +55,7 @@ describe.only('https_agent.test.js', function () {
         Object.keys(agentkeepalive.sockets).should.length(1);
         Object.keys(agentkeepalive.freeSockets).should.length(0);
         // request again
-        options.path = '/package/agentkeepalive';
+        options.path = '/search?q=agentkeepalive';
         process.nextTick(function () {
           Object.keys(agentkeepalive.sockets).should.length(1);
           Object.keys(agentkeepalive.freeSockets).should.length(1);
@@ -81,4 +81,35 @@ describe.only('https_agent.test.js', function () {
     });
   });
 
+  it('should have remove sockets after timeout', function (done) {
+    this.timeout(2500);
+    var options = {
+      hostname: 'www.google.com',
+      port: 443,
+      path: '/search?q=nodejs',
+      agent: agentkeepalive,
+    };
+    var remotePort = null;
+    agentkeepalive.get(options, function (res) {
+      Object.keys(agentkeepalive.sockets).should.length(1);
+      Object.keys(agentkeepalive.freeSockets).should.length(0);
+      res.statusCode.should.equal(200);
+      res.on('data', function (chunk) {});
+      res.on('end', function () {
+        Object.keys(agentkeepalive.sockets).should.length(1);
+        Object.keys(agentkeepalive.freeSockets).should.length(0);
+        // request again
+        options.path = '/search?q=agentkeepalive';
+        process.nextTick(function () {
+          Object.keys(agentkeepalive.sockets).should.length(1);
+          Object.keys(agentkeepalive.freeSockets).should.length(1);
+          setTimeout(function() {
+            Object.keys(agentkeepalive.sockets).should.length(0);
+            Object.keys(agentkeepalive.freeSockets).should.length(0);
+            done();
+          }, 2000);        
+        });
+      });
+    });
+  });
 });