Bug 613977 - Intermittent invalid certificate error prompt in security tests causing timeouts r=honzab a=beta-N
authorPatrick McManus <mcmanus@ducksong.com>
Wed, 05 Jan 2011 08:37:45 -0500
changeset 60015 257af9cad364
parent 60014 5875a27fcc79
child 60016 79d00f4cad73
push id17831
push usermcmanus@ducksong.com
push dateWed, 05 Jan 2011 14:18:07 +0000
treeherdermozilla-central@257af9cad364 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershonzab, beta-N
bugs613977
milestone2.0b9pre
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 613977 - Intermittent invalid certificate error prompt in security tests causing timeouts r=honzab a=beta-N
netwerk/base/src/nsSocketTransport2.cpp
netwerk/protocol/http/nsAHttpTransaction.h
netwerk/protocol/http/nsHttpConnection.cpp
netwerk/protocol/http/nsHttpConnection.h
netwerk/protocol/http/nsHttpPipeline.cpp
netwerk/protocol/http/nsHttpTransaction.cpp
--- 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/nsAHttpTransaction.h
+++ b/netwerk/protocol/http/nsAHttpTransaction.h
@@ -39,16 +39,17 @@
 #define nsAHttpTransaction_h__
 
 #include "nsISupports.h"
 
 class nsAHttpConnection;
 class nsAHttpSegmentReader;
 class nsAHttpSegmentWriter;
 class nsIInterfaceRequestor;
+class nsIEventTarget;
 
 //----------------------------------------------------------------------------
 // Abstract base class for a HTTP transaction:
 //
 // A transaction is a "sink" for the response data.  The connection pushes
 // data to the transaction by writing to it.  The transaction supports
 // WriteSegments and may refuse to accept data if its buffers are full (its
 // write function returns NS_BASE_STREAM_WOULD_BLOCK in this case).
@@ -57,17 +58,18 @@ class nsIInterfaceRequestor;
 class nsAHttpTransaction : public nsISupports
 {
 public:
     // called by the connection when it takes ownership of the transaction.
     virtual void SetConnection(nsAHttpConnection *) = 0;
 
     // called by the connection to get security callbacks to set on the 
     // socket transport.
-    virtual void GetSecurityCallbacks(nsIInterfaceRequestor **) = 0;
+    virtual void GetSecurityCallbacks(nsIInterfaceRequestor **,
+                                      nsIEventTarget **) = 0;
 
     // called to report socket status (see nsITransportEventSink)
     virtual void OnTransportStatus(nsresult status, PRUint64 progress) = 0;
 
     // called to check the transaction status.
     virtual PRBool   IsDone() = 0;
     virtual nsresult Status() = 0;
 
@@ -83,17 +85,18 @@ public:
                                    PRUint32 count, PRUint32 *countWritten) = 0;
 
     // called to close the transaction
     virtual void Close(nsresult reason) = 0;
 };
 
 #define NS_DECL_NSAHTTPTRANSACTION \
     void SetConnection(nsAHttpConnection *); \
-    void GetSecurityCallbacks(nsIInterfaceRequestor **); \
+    void GetSecurityCallbacks(nsIInterfaceRequestor **, \
+                              nsIEventTarget **);       \
     void OnTransportStatus(nsresult status, PRUint64 progress); \
     PRBool   IsDone(); \
     nsresult Status(); \
     PRUint32 Available(); \
     nsresult ReadSegments(nsAHttpSegmentReader *, PRUint32, PRUint32 *); \
     nsresult WriteSegments(nsAHttpSegmentWriter *, PRUint32, PRUint32 *); \
     void     Close(nsresult reason);
 
--- a/netwerk/protocol/http/nsHttpConnection.cpp
+++ b/netwerk/protocol/http/nsHttpConnection.cpp
@@ -46,16 +46,17 @@
 #include "nsISocketTransportService.h"
 #include "nsISocketTransport.h"
 #include "nsIServiceManager.h"
 #include "nsISSLSocketControl.h"
 #include "nsStringStream.h"
 #include "netCore.h"
 #include "nsNetCID.h"
 #include "nsAutoLock.h"
+#include "nsProxyRelease.h"
 #include "prmem.h"
 
 #ifdef DEBUG
 // defined by the socket transport service while active
 extern PRThread *gSocketThread;
 #endif
 
 static NS_DEFINE_CID(kSocketTransportServiceCID, NS_SOCKETTRANSPORTSERVICE_CID);
@@ -93,40 +94,55 @@ nsHttpConnection::nsHttpConnection()
     // grab a reference to the handler to ensure that it doesn't go away.
     nsHttpHandler *handler = gHttpHandler;
     NS_ADDREF(handler);
 }
 
 nsHttpConnection::~nsHttpConnection()
 {
     LOG(("Destroying nsHttpConnection @%x\n", this));
- 
-    if (mIdleSynTimer) {
-        mIdleSynTimer->Cancel();
-        mIdleSynTimer = nsnull;
-    }
 
+    CancelSynTimer();
     if (mBackupConnection) {
         gHttpHandler->ReclaimConnection(mBackupConnection);
         mBackupConnection = nsnull;
     }
-    
+
+    ReleaseCallbacks();
     NS_IF_RELEASE(mConnInfo);
-    NS_IF_RELEASE(mTransaction);
 
     if (mLock) {
         PR_DestroyLock(mLock);
         mLock = nsnull;
     }
 
     // release our reference to the handler
     nsHttpHandler *handler = gHttpHandler;
     NS_RELEASE(handler);
 }
 
+void
+nsHttpConnection::ReleaseCallbacks()
+{
+    if (mCallbacks) {
+        nsIInterfaceRequestor *cbs = nsnull;
+        mCallbacks.swap(cbs);
+        NS_ProxyRelease(mCallbackTarget, cbs);
+    }
+}
+
+void
+nsHttpConnection::CancelSynTimer()
+{
+    if (mIdleSynTimer) {
+        mIdleSynTimer->Cancel();
+        mIdleSynTimer = nsnull;
+    }
+}
+
 nsresult
 nsHttpConnection::Init(nsHttpConnectionInfo *info, PRUint16 maxHangTime)
 {
     LOG(("nsHttpConnection::Init [this=%x]\n", this));
 
     NS_ENSURE_ARG_POINTER(info);
     NS_ENSURE_TRUE(!mConnInfo, NS_ERROR_ALREADY_INITIALIZED);
 
@@ -154,29 +170,38 @@ nsHttpConnection::IdleSynTimeout(nsITime
     self->mIdleSynTimer = nsnull;
 
     if (!self->mSocketTransport) {
         NS_ABORT_IF_FALSE(self->mSocketTransport1 && !self->mSocketTransport2,
                           "establishing backup tranport");
 
         LOG(("SocketTransport hit idle timer - starting backup socket"));
 
+        // if we have already cleared ::CloseTransaction, then we do not
+        // need to create the backup connection
+        if (!self->mTransaction)
+            return;
+
         gHttpHandler->ConnMgr()->GetConnection(self->mConnInfo,
                                                self->mSocketCaps,
                                                getter_AddRefs(
                                                    self->mBackupConnection));
         if (!self->mBackupConnection)
             return;
         nsresult rv =
             self->CreateTransport(self->mSocketCaps,
                                   getter_AddRefs(self->mSocketTransport2),
                                   getter_AddRefs(self->mSocketIn2),
                                   getter_AddRefs(self->mSocketOut2));
         if (NS_SUCCEEDED(rv)) {
             sCreateTransport2++;
+            self->mTransaction->
+                GetSecurityCallbacks(
+                    getter_AddRefs(self->mCallbacks),
+                    getter_AddRefs(self->mCallbackTarget));
             self->mSocketOut2->AsyncWait(self, 0, 0, nsnull);
         }
     }
 
     return;
 }
 
 // called on the socket thread
@@ -189,18 +214,18 @@ nsHttpConnection::Activate(nsAHttpTransa
     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
     mTransaction = trans;
-    NS_ADDREF(mTransaction);
     mActivationCount++;
+    ReleaseCallbacks();
 
     // 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))
@@ -214,25 +239,32 @@ nsHttpConnection::Activate(nsAHttpTransa
     else {
         NS_ABORT_IF_FALSE(mSocketOut && mSocketIn,
                           "Socket Transport and SocketOut mismatch");
         
         // If this is the first transaction on this connection, but
         // we already have a socket that means the socket was created
         // speculatively in the past, not used at that time, and
         // given to the connection manager.
-        if (mActivationCount == 1)
+        if (mActivationCount == 1) {
             sWastedReuseCount++;
-
+            rv = mSocketTransport->SetEventSink(this, nsnull);
+            NS_ENSURE_SUCCESS(rv, rv);
+            rv = mSocketTransport->SetSecurityCallbacks(this);
+            NS_ENSURE_SUCCESS(rv, rv);
+        }
         rv = mSocketOut->AsyncWait(this, 0, 0, nsnull);
     }
     
 failed_activation:
-    if (NS_FAILED(rv))
-        NS_RELEASE(mTransaction);
+    if (NS_FAILED(rv)) {
+        mTransaction = nsnull;
+        CancelSynTimer();
+    }
+
     return rv;
 }
 
 void
 nsHttpConnection::Close(nsresult reason)
 {
     LOG(("nsHttpConnection::Close [this=%x reason=%x]\n", this, reason));
 
@@ -481,17 +513,17 @@ nsHttpConnection::OnHeadersAvailable(nsA
             NS_ASSERTION(NS_SUCCEEDED(rv), "mSocketOut->AsyncWait failed");
         }
         else {
             LOG(("SSL proxy CONNECT failed!\n"));
             // NOTE: this cast is valid since this connection cannot be
             // processing a transaction pipeline until after the first HTTP/1.1
             // response.
             nsHttpTransaction *trans =
-                    static_cast<nsHttpTransaction *>(mTransaction);
+                     static_cast<nsHttpTransaction *>(mTransaction.get());
             trans->SetSSLConnectFailed();
         }
     }
 
     return NS_OK;
 }
 
 void
@@ -643,20 +675,16 @@ 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);
     mSocketTransport = sock;
     mSocketOut = outs;
     mSocketIn = ins;
     return NS_OK;
 }
 
 void
 nsHttpConnection::CloseTransaction(nsAHttpTransaction *trans, nsresult reason)
@@ -667,19 +695,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;
 }
@@ -880,17 +906,18 @@ nsHttpConnection::SetupSSLProxyConnect()
     request.SetRequestURI(buf);
     request.SetHeader(nsHttp::User_Agent, gHttpHandler->UserAgent());
 
     // send this header for backwards compatibility.
     request.SetHeader(nsHttp::Proxy_Connection, NS_LITERAL_CSTRING("keep-alive"));
 
     // NOTE: this cast is valid since this connection cannot be processing a
     // transaction pipeline until after the first HTTP/1.1 response.
-    nsHttpTransaction *trans = static_cast<nsHttpTransaction *>(mTransaction);
+    nsHttpTransaction *trans =
+        static_cast<nsHttpTransaction *>(mTransaction.get());
     
     val = trans->RequestHead()->PeekHeader(nsHttp::Host);
     if (val) {
         // all HTTP/1.1 requests must include a Host header (even though it
         // may seem redundant in this case; see bug 82388).
         request.SetHeader(nsHttp::Host, nsDependentCString(val));
     }
 
@@ -925,24 +952,24 @@ 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;
-        }
-        
+
+        CancelSynTimer();
+
         if (out == mSocketOut1) {
             sSuccessTransport1++;
             mSocketTransport.swap(mSocketTransport1);
             mSocketOut.swap(mSocketOut1);
             mSocketIn.swap(mSocketIn1);
 
             if (mSocketTransport2)
                 sUnNecessaryTransport2++;
@@ -1025,28 +1052,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;
@@ -1075,18 +1102,17 @@ nsHttpConnection::OnTransportStatus(nsIT
 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) {
-        nsCOMPtr<nsIInterfaceRequestor> callbacks;
-        mTransaction->GetSecurityCallbacks(getter_AddRefs(callbacks));
-        if (callbacks)
-            return callbacks->GetInterface(iid, result);
-    }
+        
+    nsCOMPtr<nsIInterfaceRequestor> callbacks = mCallbacks;
+    if (!callbacks && mTransaction)
+        mTransaction->GetSecurityCallbacks(getter_AddRefs(callbacks), nsnull);
+    if (callbacks)
+        return callbacks->GetInterface(iid, result);
 
     return NS_ERROR_NO_INTERFACE;
 }
--- a/netwerk/protocol/http/nsHttpConnection.h
+++ b/netwerk/protocol/http/nsHttpConnection.h
@@ -48,16 +48,17 @@
 #include "prlock.h"
 #include "nsAutoPtr.h"
 
 #include "nsIStreamListener.h"
 #include "nsISocketTransport.h"
 #include "nsIAsyncInputStream.h"
 #include "nsIAsyncOutputStream.h"
 #include "nsIInterfaceRequestor.h"
+#include "nsIEventTarget.h"
 #include "nsITimer.h"
 
 //-----------------------------------------------------------------------------
 // nsHttpConnection - represents a connection to a HTTP server (or proxy)
 //
 // NOTE: this objects lives on the socket thread only.  it should not be
 // accessed from any other thread.
 //-----------------------------------------------------------------------------
@@ -149,28 +150,39 @@ private:
     PRBool   IsAlive();
     PRBool   SupportsPipelining(nsHttpResponseHead *);
     
     static void  IdleSynTimeout(nsITimer *, void *);
     void         SelectPrimaryTransport(nsIAsyncOutputStream *out);
     void         ReleaseBackupTransport(nsISocketTransport *sock,
                                         nsIAsyncOutputStream *outs,
                                         nsIAsyncInputStream *ins);
+    void         CancelSynTimer();
+    void         ReleaseCallbacks();
 private:
     nsCOMPtr<nsISocketTransport>    mSocketTransport;
     nsCOMPtr<nsIAsyncInputStream>   mSocketIn;
     nsCOMPtr<nsIAsyncOutputStream>  mSocketOut;
 
     nsresult                        mSocketInCondition;
     nsresult                        mSocketOutCondition;
 
     nsCOMPtr<nsIInputStream>        mSSLProxyConnectStream;
     nsCOMPtr<nsIInputStream>        mRequestStream;
 
-    nsAHttpTransaction             *mTransaction; // hard ref
+    // mTransaction only points to the HTTP Transaction callbacks if the
+    // transaction is open, otherwise it is null.
+    nsRefPtr<nsAHttpTransaction>    mTransaction;
+
+    // The security callbacks are only stored if we initiate a
+    // backup connection because they need to be proxy released
+    // on the main thread.
+    nsCOMPtr<nsIInterfaceRequestor> mCallbacks;
+    nsCOMPtr<nsIEventTarget>        mCallbackTarget;
+
     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=
 
--- a/netwerk/protocol/http/nsHttpPipeline.cpp
+++ b/netwerk/protocol/http/nsHttpPipeline.cpp
@@ -305,26 +305,30 @@ nsHttpPipeline::SetConnection(nsAHttpCon
     NS_IF_ADDREF(mConnection = conn);
 
     PRInt32 i, count = mRequestQ.Length();
     for (i=0; i<count; ++i)
         Request(i)->SetConnection(this);
 }
 
 void
-nsHttpPipeline::GetSecurityCallbacks(nsIInterfaceRequestor **result)
+nsHttpPipeline::GetSecurityCallbacks(nsIInterfaceRequestor **result,
+                                     nsIEventTarget        **target)
 {
     NS_ASSERTION(PR_GetCurrentThread() == gSocketThread, "wrong thread");
 
     // return security callbacks from first request
     nsAHttpTransaction *trans = Request(0);
     if (trans)
-        trans->GetSecurityCallbacks(result);
-    else
+        trans->GetSecurityCallbacks(result, target);
+    else {
         *result = nsnull;
+        if (target)
+            *target = nsnull;
+    }
 }
 
 void
 nsHttpPipeline::OnTransportStatus(nsresult status, PRUint64 progress)
 {
     LOG(("nsHttpPipeline::OnStatus [this=%x status=%x progress=%llu]\n",
         this, status, progress));
 
--- a/netwerk/protocol/http/nsHttpTransaction.cpp
+++ b/netwerk/protocol/http/nsHttpTransaction.cpp
@@ -323,19 +323,22 @@ nsHttpTransaction::TakeResponseHead()
 void
 nsHttpTransaction::SetConnection(nsAHttpConnection *conn)
 {
     NS_IF_RELEASE(mConnection);
     NS_IF_ADDREF(mConnection = conn);
 }
 
 void
-nsHttpTransaction::GetSecurityCallbacks(nsIInterfaceRequestor **cb)
+nsHttpTransaction::GetSecurityCallbacks(nsIInterfaceRequestor **cb,
+                                        nsIEventTarget        **target)
 {
     NS_IF_ADDREF(*cb = mCallbacks);
+    if (target)
+        NS_IF_ADDREF(*target = mConsumerTarget);
 }
 
 void
 nsHttpTransaction::OnTransportStatus(nsresult status, PRUint64 progress)
 {
     LOG(("nsHttpTransaction::OnSocketStatus [this=%x status=%x progress=%llu]\n",
         this, status, progress));