http client: Ensure socket cleanup on response end

If an http response has an 'end' handler that throws, then the socket
will never be released back into the pool.

Granted, we do NOT guarantee that throwing will never have adverse
effects on Node internal state.  Such a guarantee cannot be reasonably
made in a shared-global mutable-state side-effecty language like
JavaScript.  However, in this case, it's a rather trivial patch to
increase our resilience a little bit, so it seems like a win.

There is no semantic change in this case, except that some event
listeners are removed, and the `'free'` event is emitted on nextTick, so
that you can schedule another request which will re-use the same socket.
From the user's point of view, there should be no detectable difference.

Closes #5107

1 files changed, 8 insertions, 2 deletions

diff --git a/lib/http.js b/lib/http.js
index dacedd4477..1f4db7a647 100644
--- a/lib/http.js
+++ b/lib/http.js
@@ -1635,8 +1635,10 @@ function parserOnIncomingClient(res, shouldKeepAlive) {
   COUNTER_HTTP_CLIENT_RESPONSE();
   req.res = res;
   res.req = req;
-  var handled = req.emit('response', res);
+
+  // add our listener first, so that we guarantee socket cleanup
   res.on('end', responseOnEnd);
+  var handled = req.emit('response', res);
 
   // If the user did not listen for the 'response' event, then they
   // can't possibly read the data, so we ._dump() it into the void
@@ -1667,7 +1669,11 @@ function responseOnEnd() {
     }
     socket.removeListener('close', socketCloseListener);
     socket.removeListener('error', socketErrorListener);
-    socket.emit('free');
+    // Mark this socket as available, AFTER user-added end
+    // handlers have a chance to run.
+    process.nextTick(function() {
+      socket.emit('free');
+    });
   }
 }