connect-timeout callbacks after close

Makefile

@@ -97,12 +97,14 @@ test-uv: all
 		simple/test-global \
 		simple/test-http \
 		simple/test-http-1.0 \
+		simple/test-http-abort-client \
 		simple/test-http-allow-req-after-204-res \
 		simple/test-http-blank-header \
 		simple/test-http-buffer-sanity \
 		simple/test-http-cat \
 		simple/test-http-chunked \
 		simple/test-http-client-abort \
+		simple/test-http-client-parse-error \
 		simple/test-http-client-race \
 		simple/test-http-client-race-2 \
 		simple/test-http-client-upload \
@@ -143,6 +145,7 @@ test-uv: all
 		simple/test-net-binary \
 		simple/test-net-can-reset-timeout \
 		simple/test-net-connect-buffer \
+		simple/test-net-connect-timeout \
 		simple/test-net-create-connection \
 		simple/test-net-eaddrinuse \
 		simple/test-net-isip \
@@ -202,6 +205,7 @@ test-uv: all
 		simple/test-zerolengthbufferbug \
 		pummel/test-http-client-reconnect-bug \
 		pummel/test-http-upload-timeout \
+		pummel/test-net-many-clients \
 		pummel/test-net-pause \
 		pummel/test-net-pingpong-delay \
 		pummel/test-net-timeout \

lib/net_uv.js

@@ -155,6 +155,11 @@ function afterShutdown(status, handle, req) {
 
   assert.ok(self._flags & FLAG_SHUTDOWN);
 
+  // callback may come after call to destroy.
+  if (self.destroyed) {
+    return;
+  }
+
   if (self._flags & FLAG_GOT_EOF) {
     self.destroy();
   } else {
@@ -311,6 +316,10 @@ Socket.prototype.write = function(data /* [encoding], [fd], [cb] */) {
 function afterWrite(status, handle, req, buffer) {
   var self = handle.socket;
 
+  // callback may come after call to destroy.
+  if (self.destroyed) {
+    return;
+  }
   // TODO check status.
 
   var req_ = self._writeRequests.shift();
@@ -398,6 +407,11 @@ function afterConnect(status, handle, req) {
 
   debug("afterConnect");
 
+  // callback may come after call to destroy
+  if (self.destroyed) {
+    return;
+  }
+
   assert.ok(self._connecting);
   self._connecting = false;
 

src/tcp_wrap.cc

@@ -158,6 +158,13 @@ class TCPWrap {
   // Free the C++ object on the close callback.
   static void OnClose(uv_handle_t* handle) {
     TCPWrap* wrap = static_cast<TCPWrap*>(handle->data);
+
+    // The wrap object should still be there.
+    assert(wrap->object_.IsEmpty() == false);
+
+    wrap->object_->SetPointerInInternalField(0, NULL);
+    wrap->object_.Dispose();
+    wrap->object_.Clear();
     delete wrap;
   }
 
@@ -221,14 +228,12 @@ class TCPWrap {
     assert(&wrap->handle_ == (uv_tcp_t*)handle);
 
     // We should not be getting this callback if someone as already called
-    // uv_close() on the handle. Since we've destroyed object_ at the same
-    // time as calling uv_close() we can test for this here.
+    // uv_close() on the handle.
     assert(wrap->object_.IsEmpty() == false);
 
     if (status != 0) {
-      // TODO Handle server error (call onerror?)
+      // TODO Handle server error (set errno and call onconnection with NULL)
       assert(0);
-      uv_close((uv_handle_t*) handle, OnClose);
       return;
     }
 
@@ -332,8 +337,7 @@ class TCPWrap {
     TCPWrap* wrap = static_cast<TCPWrap*>(handle->data);
 
     // We should not be getting this callback if someone as already called
-    // uv_close() on the handle. Since we've destroyed object_ at the same
-    // time as calling uv_close() we can test for this here.
+    // uv_close() on the handle.
     assert(wrap->object_.IsEmpty() == false);
 
     // Remove the reference to the slab to avoid memory leaks;
@@ -373,15 +377,16 @@ class TCPWrap {
 
     UNWRAP
 
+    assert(!wrap->object_.IsEmpty());
     int r = uv_close((uv_handle_t*) &wrap->handle_, OnClose);
 
-    if (r) SetErrno(uv_last_error().code);
-
-    assert(!wrap->object_.IsEmpty());
-    wrap->object_->SetPointerInInternalField(0, NULL);
-    wrap->object_.Dispose();
-    wrap->object_.Clear();
+    if (r) {
+      SetErrno(uv_last_error().code);
 
+      wrap->object_->SetPointerInInternalField(0, NULL);
+      wrap->object_.Dispose();
+      wrap->object_.Clear();
+    }
     return scope.Close(Integer::New(r));
   }
 
@@ -391,8 +396,9 @@ class TCPWrap {
 
     HandleScope scope;
 
-    // The request object should still be there.
+    // The wrap and request objects should still be there.
     assert(req_wrap->object_.IsEmpty() == false);
+    assert(wrap->object_.IsEmpty() == false);
 
     if (status) {
       SetErrno(uv_last_error().code);
@@ -462,8 +468,9 @@ class TCPWrap {
 
     HandleScope scope;
 
-    // The request object should still be there.
+    // The wrap and request objects should still be there.
     assert(req_wrap->object_.IsEmpty() == false);
+    assert(wrap->object_.IsEmpty() == false);
 
     if (status) {
       SetErrno(uv_last_error().code);
@@ -539,8 +546,9 @@ class TCPWrap {
     ReqWrap<uv_shutdown_t>* req_wrap = (ReqWrap<uv_shutdown_t>*) req->data;
     TCPWrap* wrap = (TCPWrap*) req->handle->data;
 
-    // The request object should still be there.
+    // The wrap and request objects should still be there.
     assert(req_wrap->object_.IsEmpty() == false);
+    assert(wrap->object_.IsEmpty() == false);
 
     HandleScope scope;