bug 907800 - only retry http 408 when it is suspected of being a persistent reuse race r=jduell
authorPatrick McManus <mcmanus@ducksong.com>
Wed, 04 Sep 2013 16:39:25 -0400
changeset 145495 0783c819410df43f5dd5caa6b1db64e3ad31f109
parent 145494 15a71c6cb0d3ab794fee0b4307ef108162e66f05
child 145496 d42f8528d09f8b41bd8b15efe878eed35694a201
push id33297
push usermcmanus@ducksong.com
push dateWed, 04 Sep 2013 20:39:56 +0000
treeherdermozilla-inbound@0783c819410d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjduell
bugs907800
milestone26.0a1
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 907800 - only retry http 408 when it is suspected of being a persistent reuse race r=jduell
netwerk/protocol/http/nsHttpConnection.cpp
netwerk/protocol/http/nsHttpConnection.h
--- a/netwerk/protocol/http/nsHttpConnection.cpp
+++ b/netwerk/protocol/http/nsHttpConnection.cpp
@@ -115,17 +115,17 @@ nsHttpConnection::Init(nsHttpConnectionI
          "transport=%p instream=%p outstream=%p rtt=%d]\n",
          this, transport, instream, outstream,
          PR_IntervalToMilliseconds(rtt)));
 
     NS_ENSURE_ARG_POINTER(info);
     NS_ENSURE_TRUE(!mConnInfo, NS_ERROR_ALREADY_INITIALIZED);
 
     mConnInfo = info;
-    mLastReadTime = PR_IntervalNow();
+    mLastWriteTime = mLastReadTime = PR_IntervalNow();
     mSupportsPipelining =
         gHttpHandler->ConnMgr()->SupportsPipelining(mConnInfo);
     mRtt = rtt;
     mMaxHangTime = PR_SecondsToInterval(maxHangTime);
 
     mSocketTransport = transport;
     mSocketIn = instream;
     mSocketOut = outstream;
@@ -310,17 +310,17 @@ nsHttpConnection::Activate(nsAHttpTransa
     mPriority = pri;
     if (mTransaction && mUsingSpdyVersion)
         return AddTransaction(trans, pri);
 
     NS_ENSURE_ARG_POINTER(trans);
     NS_ENSURE_TRUE(!mTransaction, NS_ERROR_IN_PROGRESS);
 
     // reset the read timers to wash away any idle time
-    mLastReadTime = PR_IntervalNow();
+    mLastWriteTime = mLastReadTime = PR_IntervalNow();
 
     // Update security callbacks
     nsCOMPtr<nsIInterfaceRequestor> callbacks;
     trans->GetSecurityCallbacks(getter_AddRefs(callbacks));
     SetSecurityCallbacks(callbacks);
 
     SetupSSL(caps);
 
@@ -697,41 +697,51 @@ nsHttpConnection::OnHeadersAvailable(nsA
 {
     LOG(("nsHttpConnection::OnHeadersAvailable [this=%p trans=%p response-head=%p]\n",
         this, trans, responseHead));
 
     MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread);
     NS_ENSURE_ARG_POINTER(trans);
     MOZ_ASSERT(responseHead, "No response head?");
 
-    // If the server issued an explicit timeout, then we need to close down the
-    // socket transport.  We pass an error code of NS_ERROR_NET_RESET to
-    // trigger the transactions 'restart' mechanism.  We tell it to reset its
-    // response headers so that it will be ready to receive the new response.
-    uint16_t responseStatus = responseHead->Status();
-    if (responseStatus == 408) {
-        Close(NS_ERROR_NET_RESET);
-        *reset = true;
-        return NS_OK;
-    }
-
     // we won't change our keep-alive policy unless the server has explicitly
     // told us to do so.
 
     // inspect the connection headers for keep-alive info provided the
     // transaction completed successfully. In the case of a non-sensical close
     // and keep-alive favor the close out of conservatism.
 
     bool explicitKeepAlive = false;
     bool explicitClose = responseHead->HasHeaderValue(nsHttp::Connection, "close") ||
         responseHead->HasHeaderValue(nsHttp::Proxy_Connection, "close");
     if (!explicitClose)
         explicitKeepAlive = responseHead->HasHeaderValue(nsHttp::Connection, "keep-alive") ||
             responseHead->HasHeaderValue(nsHttp::Proxy_Connection, "keep-alive");
 
+    // deal with 408 Server Timeouts
+    uint16_t responseStatus = responseHead->Status();
+    static const PRIntervalTime k1000ms  = PR_MillisecondsToInterval(1000);
+    if (responseStatus == 408) {
+        // If this error could be due to a persistent connection reuse then
+        // we pass an error code of NS_ERROR_NET_RESET to
+        // trigger the transaction 'restart' mechanism.  We tell it to reset its
+        // response headers so that it will be ready to receive the new response.
+        if (mIsReused && ((PR_IntervalNow() - mLastWriteTime) < k1000ms)) {
+            Close(NS_ERROR_NET_RESET);
+            *reset = true;
+            return NS_OK;
+        }
+
+        // timeouts that are not caused by persistent connection reuse should
+        // not be retried for browser compatibility reasons. bug 907800. The
+        // server driven close is implicit in the 408.
+        explicitClose = true;
+        explicitKeepAlive = false;
+    }
+
     // reset to default (the server may have changed since we last checked)
     mSupportsPipelining = false;
 
     if ((responseHead->Version() < NS_HTTP_VERSION_1_1) ||
         (requestHead->Version() < NS_HTTP_VERSION_1_1)) {
         // HTTP/1.0 connections are by default NOT persistent
         if (explicitKeepAlive)
             mKeepAlive = true;
@@ -1214,16 +1224,17 @@ nsHttpConnection::OnReadSegment(const ch
     }
 
     nsresult rv = mSocketOut->Write(buf, count, countRead);
     if (NS_FAILED(rv))
         mSocketOutCondition = rv;
     else if (*countRead == 0)
         mSocketOutCondition = NS_BASE_STREAM_CLOSED;
     else {
+        mLastWriteTime = PR_IntervalNow();
         mSocketOutCondition = NS_OK; // reset condition
         if (!mProxyConnectInProgress)
             mTotalBytesWritten += *countRead;
     }
 
     return mSocketOutCondition;
 }
 
--- a/netwerk/protocol/http/nsHttpConnection.h
+++ b/netwerk/protocol/http/nsHttpConnection.h
@@ -209,16 +209,17 @@ private:
     nsRefPtr<nsHttpHandler>         mHttpHandler; // keep gHttpHandler alive
 
     mozilla::Mutex                  mCallbacksLock;
     nsMainThreadPtrHandle<nsIInterfaceRequestor> mCallbacks;
 
     nsRefPtr<nsHttpConnectionInfo> mConnInfo;
 
     PRIntervalTime                  mLastReadTime;
+    PRIntervalTime                  mLastWriteTime;
     PRIntervalTime                  mMaxHangTime;    // max download time before dropping keep-alive status
     PRIntervalTime                  mIdleTimeout;    // value of keep-alive: timeout=
     PRIntervalTime                  mConsiderReusedAfterInterval;
     PRIntervalTime                  mConsiderReusedAfterEpoch;
     int64_t                         mCurrentBytesRead;   // data read per activation
     int64_t                         mMaxBytesRead;       // max read in 1 activation
     int64_t                         mTotalBytesRead;     // total data read
     int64_t                         mTotalBytesWritten;  // does not include CONNECT tunnel