Bug 654201 - avoid nsHttpConnection::IsAlive() running event loop for unused SSL connections r=honzab sr=biesi

netwerk/base/public/nsISocketTransport.idl

@@ -51,7 +51,7 @@ native PRNetAddr(union PRNetAddr);
  * NOTE: This is a free-threaded interface, meaning that the methods on
  * this interface may be called from any thread.
  */
-[scriptable, uuid(19c37caa-fb41-4c32-bbf1-c6b31b75d789)]
+[scriptable, uuid(bbee8fef-6ac2-4819-9089-61169bdf5074)]
 interface nsISocketTransport : nsITransport 
 {
     /**
@@ -95,8 +95,12 @@ interface nsISocketTransport : nsITransport
    
     /**
      * Test if this socket transport is (still) connected.
+     *
+     * @param aPassive indicates that the IsAlive() method should not
+     *                 read, even non-destructively, from the network.
+     *                 SSL based transports may deadlock if called without this.
      */
-    boolean isAlive();
+    boolean isAlive(in boolean aPassive);
 
     /**
      * Socket timeouts in seconds.  To specify no timeout, pass PR_UINT32_MAX

netwerk/base/src/nsSocketTransport2.cpp

@@ -1832,7 +1832,7 @@ nsSocketTransport::SetEventSink(nsITransportEventSink *sink,
 }
 
 NS_IMETHODIMP
-nsSocketTransport::IsAlive(PRBool *result)
+nsSocketTransport::IsAlive(PRBool aPassive, PRBool *result)
 {
     *result = PR_FALSE;
 
@@ -1848,12 +1848,32 @@ nsSocketTransport::IsAlive(PRBool *result)
 
     // XXX do some idle-time based checks??
 
-    char c;
-    PRInt32 rval = PR_Recv(fd, &c, 1, PR_MSG_PEEK, 0);
+    if (aPassive) {
+        *result = PR_TRUE;                        /* presume true */
 
-    if ((rval > 0) || (rval < 0 && PR_GetError() == PR_WOULD_BLOCK_ERROR))
-        *result = PR_TRUE;
+        PRPollDesc desc;
+        desc.fd = mFD;
 
+        // include POLL_READ in the in_flags in order to take
+        // conditions PK11LoggedOut / AlreadyShutDown into account on SSL
+        // sockets as a workaround for bug 658138
+        desc.in_flags = PR_POLL_READ | PR_POLL_EXCEPT;
+        desc.out_flags = 0;
+
+        if ((PR_Poll(&desc, 1, 0) == 1) &&
+            (desc.out_flags &
+             (PR_POLL_EXCEPT | PR_POLL_ERR | PR_POLL_NVAL | PR_POLL_HUP))) {
+            *result = PR_FALSE;
+        }
+    }
+    else {
+        char c;
+        PRInt32 rval = PR_Recv(fd, &c, 1, PR_MSG_PEEK, 0);
+        
+        if ((rval > 0) || (rval < 0 && PR_GetError() == PR_WOULD_BLOCK_ERROR))
+            *result = PR_TRUE;
+    }
+    
     {
         MutexAutoLock lock(mLock);
         ReleaseFD_Locked(fd);

netwerk/protocol/ftp/nsFtpConnectionThread.cpp

@@ -1428,7 +1428,8 @@ nsFtpState::R_pasv() {
             if (NS_SUCCEEDED(rv)) {
                 if (oldPort == port) {
                     PRBool isAlive;
-                    if (NS_SUCCEEDED(strans->IsAlive(&isAlive)) && isAlive)
+                    if (NS_SUCCEEDED(strans->IsAlive(PR_FALSE, &isAlive)) &&
+                        isAlive)
                         newDataConn = PR_FALSE;
                 }
             }

netwerk/protocol/ftp/nsFtpControlConnection.cpp

@@ -116,7 +116,7 @@ nsFtpControlConnection::IsAlive()
         return PR_FALSE;
 
     PRBool isAlive = PR_FALSE;
-    mSocket->IsAlive(&isAlive);
+    mSocket->IsAlive(PR_FALSE, &isAlive);
     return isAlive;
 }
 nsresult 

netwerk/protocol/http/nsHttpConnection.cpp

@@ -76,6 +76,7 @@ nsHttpConnection::nsHttpConnection()
     , mKeepAliveMask(PR_TRUE)
     , mSupportsPipelining(PR_FALSE) // assume low-grade server
     , mIsReused(PR_FALSE)
+    , mIsActivated(PR_FALSE)
     , mCompletedProxyConnect(PR_FALSE)
     , mLastTransactionExpectedNoContent(PR_FALSE)
 {
@@ -164,6 +165,7 @@ nsHttpConnection::Activate(nsAHttpTransaction *trans, PRUint8 caps)
 
     // take ownership of the transaction
     mTransaction = trans;
+    mIsActivated = PR_TRUE;
 
     // set mKeepAlive according to what will be requested
     mKeepAliveMask = mKeepAlive = (caps & NS_HTTP_ALLOW_KEEPALIVE);
@@ -265,8 +267,15 @@ nsHttpConnection::IsAlive()
     if (!mSocketTransport)
         return PR_FALSE;
 
+    // Calling IsAlive() non passively on an SSL socket transport that has
+    // not yet completed the SSL handshake can result
+    // in the event loop being run. All code that calls
+    // nsHttpConnection::IsAlive() is not re-entrant so we need to avoid
+    // having IsAlive() trigger a real SSL read in that circumstance.
+
     PRBool alive;
-    nsresult rv = mSocketTransport->IsAlive(&alive);
+    PRBool passiveRead = mConnInfo->UsingSSL() && !mIsActivated;
+    nsresult rv = mSocketTransport->IsAlive(passiveRead, &alive);
     if (NS_FAILED(rv))
         alive = PR_FALSE;
 

netwerk/protocol/http/nsHttpConnection.h

@@ -188,6 +188,7 @@ private:
     PRPackedBool                    mKeepAliveMask;
     PRPackedBool                    mSupportsPipelining;
     PRPackedBool                    mIsReused;
+    PRPackedBool                    mIsActivated;
     PRPackedBool                    mCompletedProxyConnect;
     PRPackedBool                    mLastTransactionExpectedNoContent;
 };