From cd188b9b1950e6dd8bd78828998303efb08b835d Mon Sep 17 00:00:00 2001 From: "ukai@chromium.org" Date: Tue, 3 Aug 2010 06:44:57 +0000 Subject: Revert 54707 - Fix WebSocket crash bug. If SocketStream closes while waiting ResolveProxy, it badly calls DoResolveProxyComplete from DoLoop invoked by SocketStream::Close, and real callback of ResolveProxy failed to call SocketStream::OnIOCompleted. So, don't run DoLoop when closed, if SocketStream is calling APIs of proxy_service or resolver. Check closing_ after the API calls back and closes the SocketStream. BUG=50394,46750 TEST=websocket/tests doesn't crash Review URL: http://codereview.chromium.org/3054039 TBR=ukai@chromium.org Review URL: http://codereview.chromium.org/3017053 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@54709 0039d316-1c4b-4281-b951-d872f2087c98 --- net/socket_stream/socket_stream.cc | 38 ++++++-------------------------------- net/socket_stream/socket_stream.h | 2 -- 2 files changed, 6 insertions(+), 34 deletions(-) diff --git a/net/socket_stream/socket_stream.cc b/net/socket_stream/socket_stream.cc index 8922096..fe3afeb 100644 --- a/net/socket_stream/socket_stream.cc +++ b/net/socket_stream/socket_stream.cc @@ -188,23 +188,12 @@ void SocketStream::Close() { // of AddRef() and Release() in Connect() and Finish(), respectively. if (next_state_ == STATE_NONE) return; - MessageLoop::current()->PostTask( - FROM_HERE, - NewRunnableMethod(this, &SocketStream::DoClose)); -} - -void SocketStream::DoClose() { closing_ = true; - if (socket_.get()) - socket_->Disconnect(); - // If next_state_ is STATE_READ_WRITE, we'll run DoLoop and close - // the SocketStream. - // If next_state_ is STATE_TCP_CONNECT, it's waiting other socket establishing - // connection, so, we'll close the SocketStream now. - // In other next_state_, we'll wait for callback of other APIs, such as - // ResolveProxy(). - if (next_state_ == STATE_READ_WRITE || next_state_ == STATE_TCP_CONNECT) - DoLoop(ERR_ABORTED); + // Close asynchronously, so that delegate won't be called + // back before returning Close(). + MessageLoop::current()->PostTask( + FROM_HERE, + NewRunnableMethod(this, &SocketStream::DoLoop, OK)); } void SocketStream::RestartWithAuth( @@ -480,13 +469,6 @@ int SocketStream::DoResolveProxyComplete(int result) { // of the proxies in the returned list. return ERR_NO_SUPPORTED_PROXIES; } - if (closing_) { - // We enter STATE_CLOSE here if Close has already been called on this - // SocketStream. We put this if-clause here (not the top of the - // DoResolveProxyComplete method) to process proxy resolving error. - next_state_ = STATE_CLOSE; - return ERR_ABORTED; - } next_state_ = STATE_RESOLVE_HOST; return OK; @@ -524,7 +506,7 @@ int SocketStream::DoResolveHost() { } int SocketStream::DoResolveHostComplete(int result) { - if (result == OK && !closing_) { + if (result == OK) { next_state_ = STATE_TCP_CONNECT; result = delegate_->OnStartOpenConnection(this, &io_callback_); if (result == net::ERR_IO_PENDING) @@ -537,10 +519,6 @@ int SocketStream::DoResolveHostComplete(int result) { } int SocketStream::DoTcpConnect() { - if (closing_) { - next_state_ = STATE_CLOSE; - return ERR_ABORTED; - } next_state_ = STATE_TCP_CONNECT_COMPLETE; DCHECK(factory_); socket_.reset(factory_->CreateTCPClientSocket(addresses_, @@ -550,10 +528,6 @@ int SocketStream::DoTcpConnect() { } int SocketStream::DoTcpConnectComplete(int result) { - if (closing_) { - next_state_ = STATE_CLOSE; - return ERR_ABORTED; - } // TODO(ukai): if error occured, reconsider proxy after error. if (result != OK) { next_state_ = STATE_CLOSE; diff --git a/net/socket_stream/socket_stream.h b/net/socket_stream/socket_stream.h index ed268c2..860f785 100644 --- a/net/socket_stream/socket_stream.h +++ b/net/socket_stream/socket_stream.h @@ -220,8 +220,6 @@ class SocketStream : public base::RefCountedThreadSafe { // Used for WebSocketThrottleTest. void CopyAddrInfo(struct addrinfo* head); - void DoClose(); - // Finishes the job. // Calls OnError and OnClose of delegate, and no more // notifications will be sent to delegate. -- cgit v1.1