Bug 613977 - Intermittent invalid certificate error prompt in security tests causing timeouts, r=mcmannus, a=blocking:beta9+
☠☠ backed out by 42baa6019c31 ☠ ☠
authorHonza Bambas <honzab.moz@firemni.cz>
Wed, 22 Dec 2010 16:44:27 +0100
changeset 59607 f397877da0dd038e2b51021977c6f1b1eb814170
parent 59606 6177495526e72346ecfc1f2492f3f20ad05e1527
child 59608 14ae20ed2bd59edec01e118c90fd25f44ab8df75
child 59609 42baa6019c31930590240c4c49eea4bf231db80d
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewersmcmannus, blocking
bugs613977
milestone2.0b9pre
Bug 613977 - Intermittent invalid certificate error prompt in security tests causing timeouts, r=mcmannus, a=blocking:beta9+
netwerk/base/src/nsSocketTransport2.cpp
netwerk/protocol/http/nsHttpConnection.cpp
netwerk/protocol/http/nsHttpConnection.h
--- a/netwerk/base/src/nsSocketTransport2.cpp
+++ b/netwerk/base/src/nsSocketTransport2.cpp
@@ -1773,19 +1773,30 @@ nsSocketTransport::GetSecurityCallbacks(
     nsAutoLock lock(mLock);
     NS_IF_ADDREF(*callbacks = mCallbacks);
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSocketTransport::SetSecurityCallbacks(nsIInterfaceRequestor *callbacks)
 {
-    nsAutoLock lock(mLock);
-    mCallbacks = callbacks;
-    // XXX should we tell PSM about this?
+    nsCOMPtr<nsISupports> secinfo;
+    {
+        nsAutoLock lock(mLock);
+        mCallbacks = callbacks;
+        SOCKET_LOG(("Reset callbacks for secinfo=%p callbacks=%p\n", mSecInfo.get(), mCallbacks.get()));
+
+        secinfo = mSecInfo;
+    }
+
+    // don't call into PSM while holding mLock!!
+    nsCOMPtr<nsISSLSocketControl> secCtrl(do_QueryInterface(secinfo));
+    if (secCtrl)
+        secCtrl->SetNotificationCallbacks(callbacks);
+
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSocketTransport::SetEventSink(nsITransportEventSink *sink,
                                 nsIEventTarget *target)
 {
     nsCOMPtr<nsITransportEventSink> temp;
--- a/netwerk/protocol/http/nsHttpConnection.cpp
+++ b/netwerk/protocol/http/nsHttpConnection.cpp
@@ -81,16 +81,17 @@ nsHttpConnection::nsHttpConnection()
     , mLock(nsnull)
     , mLastReadTime(0)
     , mIdleTimeout(0)
     , mKeepAlive(PR_TRUE) // assume to keep-alive by default
     , mKeepAliveMask(PR_TRUE)
     , mSupportsPipelining(PR_FALSE) // assume low-grade server
     , mIsReused(PR_FALSE)
     , mCompletedSSLConnect(PR_FALSE)
+    , mResetCallbackOnActivation(PR_FALSE)
     , mActivationCount(0)
 {
     LOG(("Creating nsHttpConnection @%x\n", this));
 
     // grab a reference to the handler to ensure that it doesn't go away.
     nsHttpHandler *handler = gHttpHandler;
     NS_ADDREF(handler);
 }
@@ -105,17 +106,16 @@ nsHttpConnection::~nsHttpConnection()
     }
 
     if (mBackupConnection) {
         gHttpHandler->ReclaimConnection(mBackupConnection);
         mBackupConnection = nsnull;
     }
     
     NS_IF_RELEASE(mConnInfo);
-    NS_IF_RELEASE(mTransaction);
 
     if (mLock) {
         PR_DestroyLock(mLock);
         mLock = nsnull;
     }
 
     // release our reference to the handler
     nsHttpHandler *handler = gHttpHandler;
@@ -188,20 +188,29 @@ nsHttpConnection::Activate(nsAHttpTransa
     NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread");
     LOG(("nsHttpConnection::Activate [this=%x trans=%x caps=%x]\n",
          this, trans, caps));
 
     NS_ENSURE_ARG_POINTER(trans);
     NS_ENSURE_TRUE(!mTransaction, NS_ERROR_IN_PROGRESS);
 
     // take ownership of the transaction
+    mTransactionReference = trans;
     mTransaction = trans;
-    NS_ADDREF(mTransaction);
     mActivationCount++;
 
+    if (mResetCallbackOnActivation) {
+        mResetCallbackOnActivation = PR_FALSE;
+
+        rv = mSocketTransport->SetEventSink(this, nsnull);
+        NS_ENSURE_SUCCESS(rv, rv);
+        rv = mSocketTransport->SetSecurityCallbacks(this);
+        NS_ENSURE_SUCCESS(rv, rv);
+    }
+
     // set mKeepAlive according to what will be requested
     mKeepAliveMask = mKeepAlive = (caps & NS_HTTP_ALLOW_KEEPALIVE);
 
     // need to handle SSL proxy CONNECT if this is the first time.
     if (mConnInfo->UsingSSL() && mConnInfo->UsingHttpProxy() && !mCompletedSSLConnect) {
         rv = SetupSSLProxyConnect();
         if (NS_FAILED(rv))
             goto failed_activation;
@@ -221,18 +230,20 @@ nsHttpConnection::Activate(nsAHttpTransa
         // given to the connection manager.
         if (mActivationCount == 1)
             sWastedReuseCount++;
 
         rv = mSocketOut->AsyncWait(this, 0, 0, nsnull);
     }
     
 failed_activation:
-    if (NS_FAILED(rv))
-        NS_RELEASE(mTransaction);
+    if (NS_FAILED(rv)) {
+        mTransaction = nsnull;
+        mTransactionReference = nsnull;
+    }
     return rv;
 }
 
 void
 nsHttpConnection::Close(nsresult reason)
 {
     LOG(("nsHttpConnection::Close [this=%x reason=%x]\n", this, reason));
 
@@ -643,20 +654,18 @@ nsHttpConnection::CreateTransport(PRUint
     return NS_OK;
 }
 
 nsresult
 nsHttpConnection::AssignTransport(nsISocketTransport *sock,
                                   nsIAsyncOutputStream *outs,
                                   nsIAsyncInputStream *ins)
 {
-    nsresult rv = sock->SetEventSink(this, nsnull);
-    NS_ENSURE_SUCCESS(rv, rv);
-    rv =sock->SetSecurityCallbacks(this);
-    NS_ENSURE_SUCCESS(rv, rv);
+    mResetCallbackOnActivation = PR_TRUE;
+
     mSocketTransport = sock;
     mSocketOut = outs;
     mSocketIn = ins;
     return NS_OK;
 }
 
 void
 nsHttpConnection::CloseTransaction(nsAHttpTransaction *trans, nsresult reason)
@@ -667,19 +676,17 @@ nsHttpConnection::CloseTransaction(nsAHt
     NS_ASSERTION(trans == mTransaction, "wrong transaction");
     NS_ASSERTION(PR_GetCurrentThread() == gSocketThread, "wrong thread");
 
     // mask this error code because its not a real error.
     if (reason == NS_BASE_STREAM_CLOSED)
         reason = NS_OK;
 
     mTransaction->Close(reason);
-
-    NS_RELEASE(mTransaction);
-    mTransaction = 0;
+    mTransaction = nsnull;
 
     if (NS_FAILED(reason))
         Close(reason);
 
     // flag the connection as reused here for convenience sake.  certainly
     // it might be going away instead ;-)
     mIsReused = PR_TRUE;
 }
@@ -925,16 +932,19 @@ nsHttpConnection::ReleaseBackupTransport
     if (NS_FAILED(rv))
         NS_WARNING("Backup nsHttpConnection could not be reclaimed");
     mBackupConnection = nsnull;
 }
 
 void
 nsHttpConnection::SelectPrimaryTransport(nsIAsyncOutputStream *out)
 {
+    LOG(("nsHttpConnection::SelectPrimaryTransport(out=%p), mSocketOut1=%p, mSocketOut2=%p, mSocketOut=%p",
+         out, mSocketOut1.get(), mSocketOut2.get(), mSocketOut.get()));
+
     if (!mSocketOut) {
         // Setup the Main Socket
         
         if (mIdleSynTimer) {
             mIdleSynTimer->Cancel();
             mIdleSynTimer = nsnull;
         }
         
@@ -1025,28 +1035,28 @@ nsHttpConnection::OnInputStreamReady(nsI
 // nsHttpConnection::nsIOutputStreamCallback
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
 nsHttpConnection::OnOutputStreamReady(nsIAsyncOutputStream *out)
 {
     NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "wrong thread");
 
+    NS_ABORT_IF_FALSE(out == mSocketOut  ||
+                      out == mSocketOut1 ||
+                      out == mSocketOut2    , "unexpected socket");
+    if (out != mSocketOut)
+        SelectPrimaryTransport(out);
+
     // if the transaction was dropped...
     if (!mTransaction) {
         LOG(("  no transaction; ignoring event\n"));
         return NS_OK;
     }
 
-    NS_ABORT_IF_FALSE(out == mSocketOut  ||
-                      out == mSocketOut1 ||
-                      out == mSocketOut2    , "unexpected socket");
-    if (out != mSocketOut)
-        SelectPrimaryTransport(out);
-
     if (mSocketOut == out) {
         NS_ABORT_IF_FALSE(!mIdleSynTimer,"IdleSynTimer should not be set");
         nsresult rv = OnSocketWritable();
         if (NS_FAILED(rv))
             CloseTransaction(mTransaction, rv);
     }
 
     return NS_OK;
@@ -1076,17 +1086,17 @@ 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(PR_GetCurrentThread() != gSocketThread, "wrong thread");
  
-    if (mTransaction) {
+    if (mTransactionReference) {
         nsCOMPtr<nsIInterfaceRequestor> callbacks;
-        mTransaction->GetSecurityCallbacks(getter_AddRefs(callbacks));
+        mTransactionReference->GetSecurityCallbacks(getter_AddRefs(callbacks));
         if (callbacks)
             return callbacks->GetInterface(iid, result);
     }
 
     return NS_ERROR_NO_INTERFACE;
 }
--- a/netwerk/protocol/http/nsHttpConnection.h
+++ b/netwerk/protocol/http/nsHttpConnection.h
@@ -160,30 +160,34 @@ private:
     nsCOMPtr<nsIAsyncOutputStream>  mSocketOut;
 
     nsresult                        mSocketInCondition;
     nsresult                        mSocketOutCondition;
 
     nsCOMPtr<nsIInputStream>        mSSLProxyConnectStream;
     nsCOMPtr<nsIInputStream>        mRequestStream;
 
+    // mTransaction only points to mTransactionReference if the
+    // transaction is open, otherwise it is null.
+    nsRefPtr<nsAHttpTransaction>    mTransactionReference;
     nsAHttpTransaction             *mTransaction; // hard ref
     nsHttpConnectionInfo           *mConnInfo;    // hard ref
 
     PRLock                         *mLock;
 
     PRUint32                        mLastReadTime;
     PRUint16                        mMaxHangTime;    // max download time before dropping keep-alive status
     PRUint16                        mIdleTimeout;    // value of keep-alive: timeout=
 
     PRPackedBool                    mKeepAlive;
     PRPackedBool                    mKeepAliveMask;
     PRPackedBool                    mSupportsPipelining;
     PRPackedBool                    mIsReused;
     PRPackedBool                    mCompletedSSLConnect;
+    PRPackedBool                    mResetCallbackOnActivation;
 
     PRUint32                        mActivationCount;
 
     // These items are used to implement a parallel connection opening
     // attempt when network.http.connection-retry-timeout has expired
     PRUint8                         mSocketCaps;
     nsCOMPtr<nsITimer>              mIdleSynTimer;
     nsRefPtr<nsHttpConnection>      mBackupConnection;