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 id17331
push usereakhgari@mozilla.com
push dateThu, 02 Dec 2010 22:45:19 +0000
treeherdermozilla-central@2c24ce211f2b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbiesi, bsmedberg, blocking-beta8
bugs614958
milestone2.0b8pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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,