From 3a96469319291264e1caf21779cd3bfe546ceadf Mon Sep 17 00:00:00 2001 From: Henry Rawas Date: Fri, 15 Jul 2011 13:20:24 -0700 Subject: [PATCH] connect-timeout callbacks after close --- Makefile | 4 ++++ lib/net_uv.js | 14 ++++++++++++++ src/tcp_wrap.cc | 38 +++++++++++++++++++++++--------------- 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/Makefile b/Makefile index 203cb95546..001454e049 100644 --- a/Makefile +++ b/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 \ diff --git a/lib/net_uv.js b/lib/net_uv.js index cecc4c78e7..e7dafc5a7b 100644 --- a/lib/net_uv.js +++ b/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; diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index b0e5dfef5e..049f91d67b 100644 --- a/src/tcp_wrap.cc +++ b/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(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(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* req_wrap = (ReqWrap*) 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;