Sockets should never be attached to a ClientRequest before nextTick().

This way the API for interacting directly with the socket object is
consistent before and after the Agent pool is exhausted.

Fixes #1601.

1 files changed, 127 insertions, 127 deletions

diff --git a/lib/http2.js b/lib/http2.js
index ebfd3c7a26..c5a32beb94 100644
--- a/lib/http2.js
+++ b/lib/http2.js
@@ -1070,148 +1070,148 @@ ClientRequest.prototype.abort = function() {
 };
 
 ClientRequest.prototype.onSocket = function(socket) {
-  var parser = parsers.alloc();
   var req = this;
-
-  req.socket = socket;
-  req.connection = socket;
-  parser.reinitialize('response');
-  parser.socket = socket;
-  parser.incoming = null;
-  req.parser = parser;
-
-  socket._httpMessage = req;
-  // Setup "drain" propogation.
-  httpSocketSetup(socket);
-
-  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();
-    socket.destroy();
-  }
-  socket.on('error', errorListener);
-
-  socket.ondata = function(d, start, end) {
-    var ret = parser.execute(d, start, end - start);
-    if (ret instanceof Error) {
-      debug('parse error');
-      socket.destroy(ret);
-    } else if (parser.incoming && parser.incoming.upgrade) {
-      var bytesParsed = ret;
-      socket.ondata = null;
-      socket.onend = null;
-
-      var res = parser.incoming;
-      req.res = res;
-
-      // This is start + byteParsed
-      var upgradeHead = d.slice(start + bytesParsed, end);
-      if (req.listeners('upgrade').length) {
-        // Emit 'upgrade' on the Agent.
-        req.upgraded = true;
-        req.emit('upgrade', res, socket, upgradeHead);
-        socket.emit('agentRemove');
-      } else {
-        // Got upgrade header, but have no handler.
-        socket.destroy();
-      }
-    }
-  };
-
-  socket.onend = function() {
-    if (!req.res) {
-      // If we don't have a response then we know that the socket
-      // ended prematurely and we need to emit an error on the request.
-      req.emit('error', new Error("Request ended prematurely."));
+  process.nextTick(function () {
+    var parser = parsers.alloc();
+    req.socket = socket;
+    req.connection = socket;
+    parser.reinitialize('response');
+    parser.socket = socket;
+    parser.incoming = null;
+    req.parser = parser;
+
+    socket._httpMessage = req;
+    // Setup "drain" propogation.
+    httpSocketSetup(socket);
+
+    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();
+      socket.destroy();
     }
-    parser.finish();
-    parsers.free(parser); // I don't know if this is necessary --Mikeal
-    socket.destroy();
-  };
+    socket.on('error', errorListener);
+
+    socket.ondata = function(d, start, end) {
+      var ret = parser.execute(d, start, end - start);
+      if (ret instanceof Error) {
+        debug('parse error');
+        socket.destroy(ret);
+      } else if (parser.incoming && parser.incoming.upgrade) {
+        var bytesParsed = ret;
+        socket.ondata = null;
+        socket.onend = null;
+
+        var res = parser.incoming;
+        req.res = res;
+
+        // This is start + byteParsed
+        var upgradeHead = d.slice(start + bytesParsed, end);
+        if (req.listeners('upgrade').length) {
+          // Emit 'upgrade' on the Agent.
+          req.upgraded = true;
+          req.emit('upgrade', res, socket, upgradeHead);
+          socket.emit('agentRemove');
+        } else {
+          // Got upgrade header, but have no handler.
+          socket.destroy();
+        }
+      }
+    };
+
+    socket.onend = function() {
+      if (!req.res) {
+        // If we don't have a response then we know that the socket
+        // ended prematurely and we need to emit an error on the request.
+        req.emit('error', new Error("Request ended prematurely."));
+        req._hadError = true;
+      }
+      parser.finish();
+      parsers.free(parser); // I don't know if this is necessary --Mikeal
+      socket.destroy();
+    };
 
-  var closeListener = function() {
-    debug('HTTP socket close');
-    req.emit('close');
-    if (req.res && req.res.readable) {
-      // Socket closed before we emitted "end" below.
-      req.res.emit('aborted');
-      req.res.emit('end');
-      req.res.emit('close');
-    } else if (!req.res && !req._hadError) {
-      // This socket error fired before we started to
-      // receive a response. The error needs to
-      // fire on the request.
-      req.emit('error', new Error('socket hang up'));
+    var closeListener = function() {
+      debug('HTTP socket close');
+      req.emit('close');
+      if (req.res && req.res.readable) {
+        // Socket closed before we emitted "end" below.
+        req.res.emit('aborted');
+        req.res.emit('end');
+        req.res.emit('close');
+      } else if (!req.res && !req._hadError) {
+        // This socket error fired before we started to
+        // receive a response. The error needs to
+        // fire on the request.
+        req.emit('error', new Error('socket hang up'));
+      }
     }
-  }
-  socket.on('close', closeListener);
+    socket.on('close', closeListener);
 
-  parser.onIncoming = function(res, shouldKeepAlive) {
-    debug('AGENT incoming response!');
+    parser.onIncoming = function(res, shouldKeepAlive) {
+      debug('AGENT incoming response!');
 
-    if (req.res) {
-      // We already have a response object, this means the server
-      // sent a double response.
-      socket.destroy();
-      return;
-    }
-    req.res = res;
-
-    // Responses to HEAD requests are crazy.
-    // HEAD responses aren't allowed to have an entity-body
-    // but *can* have a content-length which actually corresponds
-    // to the content-length of the entity-body had the request
-    // been a GET.
-    var isHeadResponse = req.method == 'HEAD';
-    debug('AGENT isHeadResponse ' + isHeadResponse);
-
-    if (res.statusCode == 100) {
-      // restart the parser, as this is a continue message.
-      delete req.res; // Clear res so that we don't hit double-responses.
-      req.emit('continue');
-      return true;
-    }
+      if (req.res) {
+        // We already have a response object, this means the server
+        // sent a double response.
+        socket.destroy();
+        return;
+      }
+      req.res = res;
 
-    if (req.shouldKeepAlive && res.headers.connection !== 'keep-alive' && !req.upgraded) {
-      // Server MUST respond with Connection:keep-alive for us to enable it.
-      // If we've been upgraded (via WebSockets) we also shouldn't try to
-      // keep the connection open.
-      req.shouldKeepAlive = false;
-    }
+      // Responses to HEAD requests are crazy.
+      // HEAD responses aren't allowed to have an entity-body
+      // but *can* have a content-length which actually corresponds
+      // to the content-length of the entity-body had the request
+      // been a GET.
+      var isHeadResponse = req.method == 'HEAD';
+      debug('AGENT isHeadResponse ' + isHeadResponse);
+
+      if (res.statusCode == 100) {
+        // restart the parser, as this is a continue message.
+        delete req.res; // Clear res so that we don't hit double-responses.
+        req.emit('continue');
+        return true;
+      }
 
-    res.addListener('end', function() {
-      if (!req.shouldKeepAlive) {
-        if (socket.writable) {
-          debug('AGENT socket.destroySoon()');
-          socket.destroySoon();
-        }
-        assert(!socket.writable);
-      } else {
-        debug('AGENT socket keep-alive');
+      if (req.shouldKeepAlive && res.headers.connection !== 'keep-alive' && !req.upgraded) {
+        // Server MUST respond with Connection:keep-alive for us to enable it.
+        // If we've been upgraded (via WebSockets) we also shouldn't try to
+        // keep the connection open.
+        req.shouldKeepAlive = false;
       }
-    });
 
-    DTRACE_HTTP_CLIENT_RESPONSE(socket, req);
-    req.emit('response', res);
+      res.addListener('end', function() {
+        if (!req.shouldKeepAlive) {
+          if (socket.writable) {
+            debug('AGENT socket.destroySoon()');
+            socket.destroySoon();
+          }
+          assert(!socket.writable);
+        } else {
+          debug('AGENT socket keep-alive');
+        }
+      });
+
+      DTRACE_HTTP_CLIENT_RESPONSE(socket, req);
+      req.emit('response', res);
 
-    res.on('end', function() {
-      if (req.shouldKeepAlive) {
-        socket.removeListener('close', closeListener);
-        socket.removeListener('error', errorListener);
-        socket.emit('free');
-      }
-    });
+      res.on('end', function() {
+        if (req.shouldKeepAlive) {
+          socket.removeListener('close', closeListener);
+          socket.removeListener('error', errorListener);
+          socket.emit('free');
+        }
+      });
 
-    return isHeadResponse;
-  };
-  process.nextTick(function() {
+      return isHeadResponse;
+    };
     req.emit('socket', socket);
   });
+
 };
 ClientRequest.prototype._deferToConnect = function(method, arguments, cb) {
   // This function is for calls that need to happen once the socket is