Backed out changeset 38140c8a8327 to see if it fixes Bug 454896 (sporadic RLk)
authorDaniel Holbert <dholbert@cs.stanford.edu>
Thu, 11 Sep 2008 12:02:09 -0700
changeset 19169 713e17a251656ca02e552f7131ae0aa3e547b384
parent 19116 38140c8a8327cd44bffab609c00a732d7de8085b
child 19170 8e25f600225f5b47b6103112f6dcb6f40de9ea93
push id1991
push userdholbert@mozilla.com
push dateFri, 12 Sep 2008 01:03:43 +0000
treeherderautoland@8e25f600225f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs454896
milestone1.9.1b1pre
backs out38140c8a8327cd44bffab609c00a732d7de8085b
Backed out changeset 38140c8a8327 to see if it fixes Bug 454896 (sporadic RLk)
netwerk/protocol/http/src/nsHttpConnection.cpp
security/manager/boot/src/nsSecureBrowserUIImpl.cpp
security/manager/ssl/src/nsNSSIOLayer.cpp
security/manager/ssl/src/nsNSSIOLayer.h
uriloader/base/nsDocLoader.cpp
--- a/netwerk/protocol/http/src/nsHttpConnection.cpp
+++ b/netwerk/protocol/http/src/nsHttpConnection.cpp
@@ -802,17 +802,17 @@ nsHttpConnection::OnTransportStatus(nsIT
 // not called on the socket transport thread
 NS_IMETHODIMP
 nsHttpConnection::GetInterface(const nsIID &iid, void **result)
 {
     // NOTE: This function is only called on the UI thread via sync proxy from
     //       the socket transport thread.  If that weren't the case, then we'd
     //       have to worry about the possibility of mTransaction going away
     //       part-way through this function call.  See CloseTransaction.
-    NS_ASSERTION(NS_IsMainThread(), "wrong thread");
+    NS_ASSERTION(PR_GetCurrentThread() != gSocketThread, "wrong thread");
  
     if (mTransaction) {
         nsCOMPtr<nsIInterfaceRequestor> callbacks;
         mTransaction->GetSecurityCallbacks(getter_AddRefs(callbacks));
         if (callbacks)
             return callbacks->GetInterface(iid, result);
     }
 
--- a/security/manager/boot/src/nsSecureBrowserUIImpl.cpp
+++ b/security/manager/boot/src/nsSecureBrowserUIImpl.cpp
@@ -185,22 +185,23 @@ nsSecureBrowserUIImpl::~nsSecureBrowserU
   if (mTransferringRequests.ops) {
     PL_DHashTableFinish(&mTransferringRequests);
     mTransferringRequests.ops = nsnull;
   }
   if (mMonitor)
     PR_DestroyMonitor(mMonitor);
 }
 
-NS_IMPL_ISUPPORTS6(nsSecureBrowserUIImpl, nsISecureBrowserUI,
-                                          nsIWebProgressListener,
-                                          nsIFormSubmitObserver,
-                                          nsIObserver,
-                                          nsISupportsWeakReference,
-                                          nsISSLStatusProvider)
+NS_IMPL_THREADSAFE_ISUPPORTS6(nsSecureBrowserUIImpl,
+                              nsISecureBrowserUI,
+                              nsIWebProgressListener,
+                              nsIFormSubmitObserver,
+                              nsIObserver,
+                              nsISupportsWeakReference,
+                              nsISSLStatusProvider)
 
 NS_IMETHODIMP
 nsSecureBrowserUIImpl::Init(nsIDOMWindow *aWindow)
 {
 
 #ifdef PR_LOGGING
   nsCOMPtr<nsIDOMWindow> window(do_QueryReferent(mWindow));
 
--- a/security/manager/ssl/src/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/src/nsNSSIOLayer.cpp
@@ -217,25 +217,22 @@ nsNSSSocketInfo::nsNSSSocketInfo()
     mCanceled(PR_FALSE),
     mHasCleartextPhase(PR_FALSE),
     mHandshakeInProgress(PR_FALSE),
     mAllowTLSIntoleranceTimeout(PR_TRUE),
     mHandshakeStartTime(0),
     mPort(0)
 {
   mThreadData = new nsSSLSocketThreadData;
-  mCallbacksLock = nsAutoLock::NewLock("nsNSSSocketInfo::mCallbacksLock");
 }
 
 nsNSSSocketInfo::~nsNSSSocketInfo()
 {
   delete mThreadData;
 
-  nsAutoLock::DestroyLock(mCallbacksLock);
-
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return;
 
   shutdown(calledFromObject);
 }
 
 void nsNSSSocketInfo::virtualDestroyNSSReference()
@@ -326,202 +323,129 @@ void nsNSSSocketInfo::SetHasCleartextPha
 PRBool nsNSSSocketInfo::GetHasCleartextPhase()
 {
   return mHasCleartextPhase;
 }
 
 NS_IMETHODIMP
 nsNSSSocketInfo::GetNotificationCallbacks(nsIInterfaceRequestor** aCallbacks)
 {
-  nsCOMPtr<nsISupports> supports;
-  {
-    nsAutoLock lock(mCallbacksLock);
-    supports = mCallbacks;
-  }
-  nsCOMPtr<nsIInterfaceRequestor> callbacks(do_QueryInterface(supports));
-  callbacks.forget(aCallbacks);
+  *aCallbacks = mCallbacks;
+  NS_IF_ADDREF(*aCallbacks);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsNSSSocketInfo::SetNotificationCallbacks(nsIInterfaceRequestor* aCallbacks)
 {
-  nsCOMPtr<nsISupports> callbacks(do_QueryInterface(aCallbacks));
-
-  nsAutoLock lock(mCallbacksLock);
-
-  callbacks.swap(mCallbacks);
-  if (mCallbacks) {
-    mDocShellDependentStuffKnown = PR_FALSE;
+  if (!aCallbacks) {
+    mCallbacks = nsnull;
+    return NS_OK;
   }
 
+  mCallbacks = aCallbacks;
+  mDocShellDependentStuffKnown = PR_FALSE;
+
   return NS_OK;
 }
 
-class nsGatherDocshellInfoForPSMRunnable : public nsRunnable
+nsresult
+nsNSSSocketInfo::EnsureDocShellDependentStuffKnown()
 {
-public:
-  nsGatherDocshellInfoForPSMRunnable(nsIInterfaceRequestor* aCallbacks,
-                                     PRBool* aExternalReporting,
-                                     nsIX509Cert** aPreviousCert)
-  : mCallbacks(aCallbacks), mExternalReporting(aExternalReporting),
-    mPreviousCert(aPreviousCert)
-  {
-    NS_ASSERTION(aCallbacks, "Null pointer!");
-  }
-
-  NS_IMETHOD Run()
+  if (mDocShellDependentStuffKnown)
+    return NS_OK;
+
+  if (!mCallbacks || nsSSLThread::exitRequested())
+    return NS_ERROR_FAILURE;
+
+  mDocShellDependentStuffKnown = PR_TRUE;
+
+  nsCOMPtr<nsIInterfaceRequestor> proxiedCallbacks;
+  NS_GetProxyForObject(NS_PROXY_TO_MAIN_THREAD,
+                       NS_GET_IID(nsIInterfaceRequestor),
+                       static_cast<nsIInterfaceRequestor*>(mCallbacks),
+                       NS_PROXY_SYNC,
+                       getter_AddRefs(proxiedCallbacks));
+
+  // Are we running within a context that wants external SSL error reporting?
+  // We'll look at the presence of a security UI object inside docshell.
+  // If the docshell wants the lock icon, you'll get the ssl error pages, too.
+  // This is helpful to distinguish from all other contexts, like mail windows,
+  // or any other SSL connections running in the background.
+  // We must query it now and remember, because fatal SSL errors will come 
+  // with a socket close, and the socket transport might detach the callbacks 
+  // instance prior to our error reporting.
+
+  nsCOMPtr<nsIDocShell> docshell;
+
+  nsCOMPtr<nsIDocShellTreeItem> item(do_GetInterface(proxiedCallbacks));
+  if (item)
   {
-    NS_ASSERTION(NS_IsMainThread(), "Must run only on the main thread!");
-
-    *mExternalReporting = PR_FALSE;
-    *mPreviousCert = nsnull;
-
-    // Are we running within a context that wants external SSL error reporting?
-    // We'll look at the presence of a security UI object inside docshell.
-    // If the docshell wants the lock icon, you'll get the ssl error pages, too.
-    // This is helpful to distinguish from all other contexts, like mail
-    // windows, or any other SSL connections running in the background.
-    // We must query it now and remember, because fatal SSL errors will come 
-    // with a socket close, and the socket transport might detach the callbacks 
-    // instance prior to our error reporting.
-
-    nsCOMPtr<nsIDocShellTreeItem> item(do_GetInterface(mCallbacks));
-    if (!item)
-      return NS_OK;
-
+    nsCOMPtr<nsIDocShellTreeItem> proxiedItem;
     nsCOMPtr<nsIDocShellTreeItem> rootItem;
-    item->GetSameTypeRootTreeItem(getter_AddRefs(rootItem));
-
-    nsCOMPtr<nsIDocShell> docshell(do_QueryInterface(rootItem));
+    NS_GetProxyForObject(NS_PROXY_TO_MAIN_THREAD,
+                         NS_GET_IID(nsIDocShellTreeItem),
+                         item.get(),
+                         NS_PROXY_SYNC,
+                         getter_AddRefs(proxiedItem));
+
+    proxiedItem->GetSameTypeRootTreeItem(getter_AddRefs(rootItem));
+    docshell = do_QueryInterface(rootItem);
     NS_ASSERTION(docshell, "rootItem do_QI is null");
-    if (!docshell)
-      return NS_OK;
-
-    nsCOMPtr<nsISecureBrowserUI> secureUI;
-    docshell->GetSecurityUI(getter_AddRefs(secureUI));
-    if (!secureUI)
-      return NS_OK;
-
-    *mExternalReporting = PR_TRUE;
-
-    // If this socket is associated to a docshell, let's try to remember
-    // the currently used cert. If this socket gets a notification from NSS
-    // having the same raw socket, we can keep the PSM wrapper object
-    // and all the data it has cached (like verification results).
-    nsCOMPtr<nsISSLStatusProvider> statprov(do_QueryInterface(secureUI));
-    if (!statprov)
-      return NS_OK;
-
-    nsCOMPtr<nsISupports> isup_stat;
-    statprov->GetSSLStatus(getter_AddRefs(isup_stat));
-
-    nsCOMPtr<nsISSLStatus> sslstat(do_QueryInterface(isup_stat));
-    if (!sslstat)
-      return NS_OK;
-
-    sslstat->GetServerCert(mPreviousCert);
-    return NS_OK;
   }
 
-private:
-  nsIInterfaceRequestor* mCallbacks;
-  PRBool* mExternalReporting;
-  nsIX509Cert** mPreviousCert;
-};
-
-nsresult
-nsNSSSocketInfo::EnsureDocShellDependentStuffKnown(PRBool* aExternalReporting,
-                                                   nsIX509Cert** aPreviousCert)
-{
-  do {
-    nsCOMPtr<nsISupports> origCallbacks;
+  if (docshell)
+  {
+    nsCOMPtr<nsIDocShell> proxiedDocShell;
+    NS_GetProxyForObject(NS_PROXY_TO_MAIN_THREAD,
+                         NS_GET_IID(nsIDocShell),
+                         docshell.get(),
+                         NS_PROXY_SYNC,
+                         getter_AddRefs(proxiedDocShell));
+    nsISecureBrowserUI* secureUI;
+    proxiedDocShell->GetSecurityUI(&secureUI);
+    if (secureUI)
     {
-      nsAutoLock lock(mCallbacksLock);
-
-      if (mDocShellDependentStuffKnown) {
-        if (aExternalReporting) {
-          *aExternalReporting = mExternalErrorReporting;
-        }
-        if (aPreviousCert) {
-          NS_IF_ADDREF(*aPreviousCert = mPreviousCert);
+      nsCOMPtr<nsIThread> mainThread(do_GetMainThread());
+      NS_ProxyRelease(mainThread, secureUI, PR_FALSE);
+      mExternalErrorReporting = PR_TRUE;
+
+      // If this socket is associated to a docshell, let's try to remember
+      // the currently used cert. If this socket gets a notification from NSS
+      // having the same raw socket, we can keep the PSM wrapper object
+      // and all the data it has cached (like verification results).
+      nsCOMPtr<nsISSLStatusProvider> statprov = do_QueryInterface(secureUI);
+      if (statprov) {
+        nsCOMPtr<nsISupports> isup_stat;
+        statprov->GetSSLStatus(getter_AddRefs(isup_stat));
+        if (isup_stat) {
+          nsCOMPtr<nsISSLStatus> sslstat = do_QueryInterface(isup_stat);
+          if (sslstat) {
+            sslstat->GetServerCert(getter_AddRefs(mPreviousCert));
+          }
         }
-        return NS_OK;
-      }
-
-      origCallbacks = mCallbacks;
-    }
-
-    if (nsSSLThread::exitRequested() || !origCallbacks) {
-      return NS_ERROR_FAILURE;
-    }
-
-    nsCOMPtr<nsIInterfaceRequestor> callbacks(do_QueryInterface(origCallbacks));
-    NS_ASSERTION(callbacks, "How does this not QI to nsIInterfaceRequestor?!");
-
-    // We're about to touch the docshell which we know to be main-thread-only.
-    nsCOMPtr<nsIThread> mainThread(do_GetMainThread());
-    NS_ENSURE_TRUE(mainThread, NS_ERROR_FAILURE);
-
-    PRBool externalReporting = PR_FALSE;
-    nsCOMPtr<nsIX509Cert> previousCert;
-
-    nsCOMPtr<nsIRunnable> runnable =
-      new nsGatherDocshellInfoForPSMRunnable(callbacks, &externalReporting,
-                                             getter_AddRefs(previousCert));
-    NS_ENSURE_TRUE(runnable, NS_ERROR_OUT_OF_MEMORY);
-
-    nsresult rv = mainThread->Dispatch(runnable, NS_DISPATCH_SYNC);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    {
-      nsAutoLock lock(mCallbacksLock);
-
-      // Check to see if anyone replaced mCallbacks while the runnable was
-      // queued. This is why we store nsISupports in mCallbacks - otherwise we'd
-      // have to QI inside this lock.
-      if (mCallbacks == origCallbacks) {
-        // No one reset this out from under us, so go ahead and update our data.
-        mDocShellDependentStuffKnown = PR_TRUE;
-        mExternalErrorReporting = externalReporting;
-        previousCert.swap(mPreviousCert);
-
-        if (aExternalReporting) {
-          *aExternalReporting = mExternalErrorReporting;
-        }
-        if (aPreviousCert) {
-          NS_IF_ADDREF(*aPreviousCert = mPreviousCert);
-        }
-        return NS_OK;
       }
     }
-
-    // Someone replaced mCallbacks while we were running, try again.
-    NS_WARNING("Contention for mCallbacks, trying again");
-
-  } while(1); // Loop until we don't have contention.
-
-  NS_NOTREACHED("Should never get here");
-  return NS_ERROR_UNEXPECTED;
+  }
+
+  return NS_OK;
 }
 
 nsresult
 nsNSSSocketInfo::GetExternalErrorReporting(PRBool* state)
 {
-  NS_ENSURE_ARG_POINTER(state);
-  nsresult rv = EnsureDocShellDependentStuffKnown(state);
+  nsresult rv = EnsureDocShellDependentStuffKnown();
   NS_ENSURE_SUCCESS(rv, rv);
-
+  *state = mExternalErrorReporting;
   return NS_OK;
 }
 
 nsresult
 nsNSSSocketInfo::SetExternalErrorReporting(PRBool aState)
 {
-  nsAutoLock lock(mCallbacksLock);
   mExternalErrorReporting = aState;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsNSSSocketInfo::GetSecurityState(PRUint32* state)
 {
   *state = mSecurityState;
@@ -615,43 +539,37 @@ nsresult
 nsNSSSocketInfo::SetErrorMessage(const PRUnichar* aText) {
   mErrorMessage.Assign(aText);
   return NS_OK;
 }
 
 /* void getInterface (in nsIIDRef uuid, [iid_is (uuid), retval] out nsQIResult result); */
 NS_IMETHODIMP nsNSSSocketInfo::GetInterface(const nsIID & uuid, void * *result)
 {
-  nsCOMPtr<nsISupports> callbacks;
-
-  {
-    nsAutoLock lock(mCallbacksLock);
-    callbacks = mCallbacks;
-  }
-
-  if (callbacks) {
-    // XXX Shouldn't we check this even if callbacks is null?
+  nsresult rv;
+  if (!mCallbacks) {
+    nsCOMPtr<nsIInterfaceRequestor> ir = new PipUIContext();
+    if (!ir)
+      return NS_ERROR_OUT_OF_MEMORY;
+
+    rv = ir->GetInterface(uuid, result);
+  } else {
     if (nsSSLThread::exitRequested())
       return NS_ERROR_FAILURE;
 
     nsCOMPtr<nsIInterfaceRequestor> proxiedCallbacks;
-    nsresult rv = NS_GetProxyForObject(NS_PROXY_TO_MAIN_THREAD,
-                                       NS_GET_IID(nsIInterfaceRequestor),
-                                       callbacks,
-                                       NS_PROXY_SYNC,
-                                       getter_AddRefs(proxiedCallbacks));
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    return proxiedCallbacks->GetInterface(uuid, result);
+    NS_GetProxyForObject(NS_PROXY_TO_MAIN_THREAD,
+                         NS_GET_IID(nsIInterfaceRequestor),
+                         mCallbacks,
+                         NS_PROXY_SYNC,
+                         getter_AddRefs(proxiedCallbacks));
+
+    rv = proxiedCallbacks->GetInterface(uuid, result);
   }
-
-  nsCOMPtr<nsIInterfaceRequestor> ir = new PipUIContext();
-  NS_ENSURE_TRUE(ir, NS_ERROR_OUT_OF_MEMORY);
-
-  return ir->GetInterface(uuid, result);
+  return rv;
 }
 
 nsresult
 nsNSSSocketInfo::GetForSTARTTLS(PRBool* aForSTARTTLS)
 {
   *aForSTARTTLS = mForSTARTTLS;
   return NS_OK;
 }
@@ -790,19 +708,22 @@ nsresult nsNSSSocketInfo::SetFileDescPtr
 {
   mFd = aFilePtr;
   return NS_OK;
 }
 
 nsresult nsNSSSocketInfo::GetPreviousCert(nsIX509Cert** _result)
 {
   NS_ENSURE_ARG_POINTER(_result);
-  nsresult rv = EnsureDocShellDependentStuffKnown(nsnull, _result);
+  nsresult rv = EnsureDocShellDependentStuffKnown();
   NS_ENSURE_SUCCESS(rv, rv);
 
+  *_result = mPreviousCert;
+  NS_IF_ADDREF(*_result);
+
   return NS_OK;
 }
 
 nsresult nsNSSSocketInfo::GetCert(nsIX509Cert** _result)
 {
   NS_ENSURE_ARG_POINTER(_result);
 
   *_result = mCert;
--- a/security/manager/ssl/src/nsNSSIOLayer.h
+++ b/security/manager/ssl/src/nsNSSIOLayer.h
@@ -38,17 +38,16 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
 #ifndef _NSNSSIOLAYER_H
 #define _NSNSSIOLAYER_H
 
 #include "prtypes.h"
 #include "prio.h"
-#include "prlock.h"
 #include "certt.h"
 #include "nsString.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsITransportSecurityInfo.h"
 #include "nsISSLSocketControl.h"
 #include "nsSSLStatus.h"
 #include "nsISSLStatusProvider.h"
@@ -197,17 +196,17 @@ public:
   /* Set SSL Status values */
   nsresult SetSSLStatus(nsSSLStatus *aSSLStatus);
   nsSSLStatus* SSLStatus() { return mSSLStatus; }
   PRBool hasCertErrors();
   
   PRStatus CloseSocketAndDestroy();
   
 protected:
-  nsCOMPtr<nsISupports> mCallbacks;
+  nsCOMPtr<nsIInterfaceRequestor> mCallbacks;
   PRFileDesc* mFd;
   nsCOMPtr<nsIX509Cert> mCert;
   nsCOMPtr<nsIX509Cert> mPreviousCert; // DocShellDependent
   enum { 
     blocking_state_unknown, is_nonblocking_socket, is_blocking_socket 
   } mBlockingState;
   PRUint32 mSecurityState;
   PRInt32 mSubRequestsHighSecurity;
@@ -231,23 +230,17 @@ protected:
 
   /* SSL Status */
   nsRefPtr<nsSSLStatus> mSSLStatus;
 
   nsresult ActivateSSL();
 
   nsSSLSocketThreadData *mThreadData;
 
-  // This lock will protect mCallbacks, mDocShellDependentStuffKnown,
-  // mExternalErrorReporting, and mPreviousCert from concurrent changes.
-  PRLock* mCallbacksLock;
-
-  nsresult
-  EnsureDocShellDependentStuffKnown(PRBool* aExternalReporting = nsnull,
-                                    nsIX509Cert** aPreviousCert = nsnull);
+  nsresult EnsureDocShellDependentStuffKnown();
 
 private:
   virtual void virtualDestroyNSSReference();
   void destructorSafeDestroyNSSReference();
 
 friend class nsSSLThread;
 };
 
--- a/uriloader/base/nsDocLoader.cpp
+++ b/uriloader/base/nsDocLoader.cpp
@@ -214,18 +214,18 @@ nsDocLoader::~nsDocLoader()
     PL_DHashTableFinish(&mRequestInfoHash);
   }
 }
 
 
 /*
  * Implementation of ISupports methods...
  */
-NS_IMPL_ADDREF(nsDocLoader)
-NS_IMPL_RELEASE(nsDocLoader)
+NS_IMPL_THREADSAFE_ADDREF(nsDocLoader)
+NS_IMPL_THREADSAFE_RELEASE(nsDocLoader)
 
 NS_INTERFACE_MAP_BEGIN(nsDocLoader)
    NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIRequestObserver)
    NS_INTERFACE_MAP_ENTRY(nsIRequestObserver)
    NS_INTERFACE_MAP_ENTRY(nsIDocumentLoader)
    NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference)
    NS_INTERFACE_MAP_ENTRY(nsIWebProgress)
    NS_INTERFACE_MAP_ENTRY(nsIProgressEventSink)