Bug 614958 - Don't release the socket transport's event sink while we're holding a lock, since it can result in retrying to obtain the same lock again, which causes a deadlock; r=biesi,bsmedberg a=blocking-beta8+ landed on a CLOSED TREE
authorEhsan Akhgari <ehsan@mozilla.com>
Thu, 02 Dec 2010 16:29:09 -0500
changeset 58513 2c24ce211f2b7ee5223adec98c82b56178053b03
parent 58512 5ae1f2fa0d9f2b9560d5aecc28d4b945a78db941
child 58514 7ff5dc0e487bd341affdf21603cdf576578bb15e
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewersbiesi, bsmedberg, blocking-beta8
bugs614958
milestone2.0b8pre
Bug 614958 - Don't release the socket transport's event sink while we're holding a lock, since it can result in retrying to obtain the same lock again, which causes a deadlock; r=biesi,bsmedberg a=blocking-beta8+ landed on a CLOSED TREE
netwerk/base/src/nsSocketTransport2.cpp
--- a/netwerk/base/src/nsSocketTransport2.cpp
+++ b/netwerk/base/src/nsSocketTransport2.cpp
@@ -1607,32 +1607,38 @@ nsSocketTransport::OnSocketDetached(PRFi
     // bug 285991 for details.
     nsCOMPtr<nsISSLSocketControl> secCtrl = do_QueryInterface(mSecInfo);
     if (secCtrl)
         secCtrl->SetNotificationCallbacks(nsnull);
 
     // finally, release our reference to the socket (must do this within
     // the transport lock) possibly closing the socket. Also release our
     // listeners to break potential refcount cycles.
+
+    // We should be careful not to release mEventSink while we're locked,
+    // because releasing it might require acquiring the lock again, so
+    // we just null out mEventSink while we're holding the lock, and let
+    // ourEventSink's destuctor take care of destroying it if needed.
+    nsCOMPtr<nsITransportEventSink> ourEventSink;
     {
         nsAutoLock lock(mLock);
         if (mFD) {
             ReleaseFD_Locked(mFD);
             // flag mFD as unusable; this prevents other consumers from 
             // acquiring a reference to mFD.
             mFDconnected = PR_FALSE;
         }
 
         // We must release mCallbacks and mEventSink to avoid memory leak
         // but only when RecoverFromError() above failed. Otherwise we lose
         // link with UI and security callbacks on next connection attempt 
         // round. That would lead e.g. to a broken certificate exception page.
         if (NS_FAILED(mCondition)) {
             mCallbacks = nsnull;
-            mEventSink = nsnull;
+            mEventSink.swap(ourEventSink);
         }
     }
 }
 
 //-----------------------------------------------------------------------------
 // xpcom api
 
 NS_IMPL_THREADSAFE_ISUPPORTS4(nsSocketTransport,