Bug 1360570 - Make sure to write as much data as possible during FastOpen. r=mcmanus
authorDragana Damjanovic <dd.mozilla@gmail.com>
Thu, 04 May 2017 12:15:23 +0200
changeset 572787 caf4f16f04370968e7219dc885e0578589f8dcf2
parent 572786 4a3896a7e29f605d0bc509ca98095c1960aa70cf
child 572788 352f22e37dc37d0a3f613a5498f90e9825f6ec60
push id57195
push userbmo:rbarker@mozilla.com
push dateThu, 04 May 2017 20:08:56 +0000
reviewersmcmanus
bugs1360570
milestone55.0a1
Bug 1360570 - Make sure to write as much data as possible during FastOpen. r=mcmanus
netwerk/protocol/http/Http2Session.cpp
netwerk/protocol/http/nsHttpConnection.cpp
netwerk/protocol/http/nsHttpConnection.h
--- a/netwerk/protocol/http/Http2Session.cpp
+++ b/netwerk/protocol/http/Http2Session.cpp
@@ -497,17 +497,18 @@ Http2Session::NetworkRead(nsAHttpSegment
   if (NS_SUCCEEDED(rv) && *countWritten > 0)
     mLastReadEpoch = PR_IntervalNow();
   return rv;
 }
 
 void
 Http2Session::SetWriteCallbacks()
 {
-  if (mConnection && (GetWriteQueueSize() || mOutputQueueUsed)) {
+  if (mConnection &&
+      (GetWriteQueueSize() || (mOutputQueueUsed > mOutputQueueSent))) {
     Unused << mConnection->ResumeSend();
   }
 }
 
 void
 Http2Session::RealignOutputQueue()
 {
   if (mAttemptingEarlyData) {
--- a/netwerk/protocol/http/nsHttpConnection.cpp
+++ b/netwerk/protocol/http/nsHttpConnection.cpp
@@ -85,16 +85,18 @@ nsHttpConnection::nsHttpConnection()
     , mWaitingFor0RTTResponse(false)
     , mContentBytesWritten0RTT(0)
     , mEarlyDataNegotiated(false)
     , mDid0RTTSpdy(false)
     , mResponseThrottled(false)
     , mResumeRecvOnUnthrottle(false)
     , mFastOpen(nullptr)
     , mFastOpenStatus(TFO_NOT_TRIED)
+    , mForceSendDuringFastOpenPending(false)
+    , mReceivedSocketWouldBlockDuringFastOpen(false)
 {
     LOG(("Creating nsHttpConnection @%p\n", this));
 
     // the default timeout is for when this connection has not yet processed a
     // transaction
     static const PRIntervalTime k5Sec = PR_SecondsToInterval(5);
     mIdleTimeout =
         (k5Sec < gHttpHandler->IdleTimeout()) ? k5Sec : gHttpHandler->IdleTimeout();
@@ -458,16 +460,19 @@ nsHttpConnection::EnsureNPNComplete(nsre
                     segmentSize, &aOut0RTTBytesWritten);
                 if (NS_FAILED(aOut0RTTWriteHandshakeValue) &&
                     aOut0RTTWriteHandshakeValue != NS_BASE_STREAM_WOULD_BLOCK) {
                     goto npnComplete;
                 }
                 LOG(("nsHttpConnection::EnsureNPNComplete [this=%p] - written %d "
                      "bytes during 0RTT", this, aOut0RTTBytesWritten));
                 mContentBytesWritten0RTT += aOut0RTTBytesWritten;
+                if (mSocketOutCondition == NS_BASE_STREAM_WOULD_BLOCK) {
+                    mReceivedSocketWouldBlockDuringFastOpen = true;
+                }
             }
         }
 
         rv = ssl->DriveHandshake();
         if (NS_FAILED(rv) && rv != NS_BASE_STREAM_WOULD_BLOCK) {
             goto npnComplete;
         }
 
@@ -1384,25 +1389,84 @@ nsHttpConnection::PushBack(const char *d
         NS_ERROR("nsHttpConnection::PushBack only one buffer supported");
         return NS_ERROR_UNEXPECTED;
     }
 
     mInputOverflow = new nsPreloadedStream(mSocketIn, data, length);
     return NS_OK;
 }
 
+class HttpConnectionForceIO : public Runnable
+{
+public:
+  HttpConnectionForceIO(nsHttpConnection *aConn, bool doRecv,
+                        bool isFastOpenForce)
+     : mConn(aConn)
+     , mDoRecv(doRecv)
+     , mIsFastOpenForce(isFastOpenForce)
+    {}
+
+    NS_IMETHOD Run() override
+    {
+        MOZ_ASSERT(OnSocketThread(), "not on socket thread");
+
+        if (mDoRecv) {
+            if (!mConn->mSocketIn)
+                return NS_OK;
+            return mConn->OnInputStreamReady(mConn->mSocketIn);
+        }
+
+        // This runnable will be called when the ForceIO timer expires
+        // (mIsFastOpenForce==false) or during the TCP Fast Open to force
+        // writes (mIsFastOpenForce==true).
+        if (mIsFastOpenForce && !mConn->mWaitingFor0RTTResponse) {
+            // If we have exit the TCP Fast Open in the meantime we can skip
+            // this.
+            return NS_OK;
+        }
+        if (!mIsFastOpenForce) {
+            MOZ_ASSERT(mConn->mForceSendPending);
+            mConn->mForceSendPending = false;
+        }
+
+        if (!mConn->mSocketOut) {
+            return NS_OK;
+        }
+        return mConn->OnOutputStreamReady(mConn->mSocketOut);
+    }
+private:
+    RefPtr<nsHttpConnection> mConn;
+    bool mDoRecv;
+    bool mIsFastOpenForce;
+};
+
 nsresult
 nsHttpConnection::ResumeSend()
 {
     LOG(("nsHttpConnection::ResumeSend [this=%p]\n", this));
 
     MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
-    if (mSocketOut)
-        return mSocketOut->AsyncWait(this, 0, 0, nullptr);
+    if (mSocketOut) {
+        nsresult rv = mSocketOut->AsyncWait(this, 0, 0, nullptr);
+        LOG(("nsHttpConnection::ResumeSend [this=%p] "
+             "mWaitingFor0RTTResponse=%d mForceSendDuringFastOpenPending=%d "
+             "mReceivedSocketWouldBlockDuringFastOpen=%d\n",
+             this, mWaitingFor0RTTResponse, mForceSendDuringFastOpenPending,
+             mReceivedSocketWouldBlockDuringFastOpen));
+        if (mWaitingFor0RTTResponse && !mForceSendDuringFastOpenPending &&
+            !mReceivedSocketWouldBlockDuringFastOpen &&
+            NS_SUCCEEDED(rv)) {
+            // During TCP Fast Open, poll does not work properly so we will
+            // trigger writes manually.
+            mForceSendDuringFastOpenPending = true;
+            NS_DispatchToCurrentThread(new HttpConnectionForceIO(this, false, true));
+        }
+        return rv;
+    }
 
     NS_NOTREACHED("no socket output stream");
     return NS_ERROR_UNEXPECTED;
 }
 
 nsresult
 nsHttpConnection::ResumeRecv()
 {
@@ -1435,55 +1499,24 @@ nsHttpConnection::ResumeRecv()
 
     if (mSocketIn)
         return mSocketIn->AsyncWait(this, 0, 0, nullptr);
 
     NS_NOTREACHED("no socket input stream");
     return NS_ERROR_UNEXPECTED;
 }
 
-
-class HttpConnectionForceIO : public Runnable
-{
-public:
-  HttpConnectionForceIO(nsHttpConnection *aConn, bool doRecv)
-     : mConn(aConn)
-     , mDoRecv(doRecv)
-    {}
-
-    NS_IMETHOD Run() override
-    {
-        MOZ_ASSERT(OnSocketThread(), "not on socket thread");
-
-        if (mDoRecv) {
-            if (!mConn->mSocketIn)
-                return NS_OK;
-            return mConn->OnInputStreamReady(mConn->mSocketIn);
-        }
-
-        MOZ_ASSERT(mConn->mForceSendPending);
-        mConn->mForceSendPending = false;
-        if (!mConn->mSocketOut) {
-            return NS_OK;
-        }
-        return mConn->OnOutputStreamReady(mConn->mSocketOut);
-    }
-private:
-    RefPtr<nsHttpConnection> mConn;
-    bool mDoRecv;
-};
-
 void
 nsHttpConnection::ForceSendIO(nsITimer *aTimer, void *aClosure)
 {
     MOZ_ASSERT(OnSocketThread(), "not on socket thread");
     nsHttpConnection *self = static_cast<nsHttpConnection *>(aClosure);
     MOZ_ASSERT(aTimer == self->mForceSendTimer);
     self->mForceSendTimer = nullptr;
-    NS_DispatchToCurrentThread(new HttpConnectionForceIO(self, false));
+    NS_DispatchToCurrentThread(new HttpConnectionForceIO(self, false, false));
 }
 
 nsresult
 nsHttpConnection::MaybeForceSendIO()
 {
     MOZ_ASSERT(OnSocketThread(), "not on socket thread");
     // due to bug 1213084 sometimes real I/O events do not get serviced when
     // NSPR derived I/O events are ready and this can cause a deadlock with
@@ -1504,17 +1537,17 @@ nsHttpConnection::MaybeForceSendIO()
 
 // trigger an asynchronous read
 nsresult
 nsHttpConnection::ForceRecv()
 {
     LOG(("nsHttpConnection::ForceRecv [this=%p]\n", this));
     MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
-    return NS_DispatchToCurrentThread(new HttpConnectionForceIO(this, true));
+    return NS_DispatchToCurrentThread(new HttpConnectionForceIO(this, true, false));
 }
 
 // trigger an asynchronous write
 nsresult
 nsHttpConnection::ForceSend()
 {
     LOG(("nsHttpConnection::ForceSend [this=%p]\n", this));
     MOZ_ASSERT(OnSocketThread(), "not on socket thread");
@@ -1660,16 +1693,18 @@ nsHttpConnection::OnSocketWritable()
     nsresult rv;
     uint32_t transactionBytes;
     bool again = true;
 
     // Prevent STS thread from being blocked by single OnOutputStreamReady callback.
     const uint32_t maxWriteAttempts = 128;
     uint32_t writeAttempts = 0;
 
+    mForceSendDuringFastOpenPending = false;
+
     do {
         ++writeAttempts;
         rv = mSocketOutCondition = NS_OK;
         transactionBytes = 0;
 
         // The SSL handshake must be completed before the transaction->readsegments()
         // processing can proceed because we need to know how to format the
         // request differently for http/1, http/2, spdy, etc.. and that is
@@ -1706,51 +1741,61 @@ nsHttpConnection::OnSocketWritable()
             LOG(("  writing transaction request stream\n"));
             mProxyConnectInProgress = false;
             rv = mTransaction->ReadSegmentsAgain(this, nsIOService::gDefaultSegmentSize,
                                                  &transactionBytes, &again);
             mContentBytesWritten += transactionBytes;
         }
 
         LOG(("nsHttpConnection::OnSocketWritable %p "
-             "ReadSegments returned [rv=%" PRIx32 " read=%u sock-cond=%" PRIx32 "]\n",
+             "ReadSegments returned [rv=%" PRIx32 " read=%u "
+             "sock-cond=%" PRIx32 " again=%d]\n",
              this, static_cast<uint32_t>(rv), transactionBytes,
-             static_cast<uint32_t>(mSocketOutCondition)));
+             static_cast<uint32_t>(mSocketOutCondition), again));
 
         // XXX some streams return NS_BASE_STREAM_CLOSED to indicate EOF.
         if (rv == NS_BASE_STREAM_CLOSED && !mTransaction->IsDone()) {
             rv = NS_OK;
             transactionBytes = 0;
         }
 
+        if (!again && (mFastOpen || mWaitingFor0RTTResponse)) {
+            // Continue waiting;
+            rv = mSocketOut->AsyncWait(this, 0, 0, nullptr);
+        }
         if (NS_FAILED(rv)) {
             // if the transaction didn't want to write any more data, then
             // wait for the transaction to call ResumeSend.
-            if (rv == NS_BASE_STREAM_WOULD_BLOCK)
+            if (rv == NS_BASE_STREAM_WOULD_BLOCK) {
                 rv = NS_OK;
+                if (mFastOpen || mWaitingFor0RTTResponse) {
+                    // Continue waiting;
+                    rv = mSocketOut->AsyncWait(this, 0, 0, nullptr);
+                }
+            }
             again = false;
         } else if (NS_FAILED(mSocketOutCondition)) {
             if (mSocketOutCondition == NS_BASE_STREAM_WOULD_BLOCK) {
                 if (mTLSFilter) {
                     LOG(("  blocked tunnel (handshake?)\n"));
                     rv = mTLSFilter->NudgeTunnel(this);
                 } else {
                     rv = mSocketOut->AsyncWait(this, 0, 0, nullptr); // continue writing
                 }
             } else {
                 rv = mSocketOutCondition;
             }
             again = false;
-        } else if (mFastOpen) {
-            rv = mSocketOut->AsyncWait(this, 0, 0, nullptr); // wait for connected event.
-            again = false;
         } else if (!transactionBytes) {
             rv = NS_OK;
 
-            if (mTransaction && !mWaitingFor0RTTResponse) { // in case the ReadSegments stack called CloseTransaction()
+            if (mWaitingFor0RTTResponse || mFastOpen) {
+                // Wait for tls handshake to finish or waiting for connect.
+                rv = mSocketOut->AsyncWait(this, 0, 0, nullptr);
+            } else if (mTransaction) { // in case the ReadSegments stack called CloseTransaction()
                 //
                 // at this point we've written out the entire transaction, and now we
                 // must wait for the server's response.  we manufacture a status message
                 // here to reflect the fact that we are waiting.  this message will be
                 // trumped (overwritten) if the server responds quickly.
                 //
                 mTransaction->OnTransportStatus(mSocketTransport,
                                                 NS_NET_STATUS_WAITING_FOR,
--- a/netwerk/protocol/http/nsHttpConnection.h
+++ b/netwerk/protocol/http/nsHttpConnection.h
@@ -400,16 +400,19 @@ private:
     // Accessed only on the socket thread.
     bool                           mResponseThrottled;
     // A read from the socket was requested while we where throttled, means
     // to ResumeRecv() when untrottled again. Only accessed on the socket thread.
     bool                           mResumeRecvOnUnthrottle;
 
     PRFileDesc                    *mFastOpen;
     uint8_t                        mFastOpenStatus;
+
+    bool                           mForceSendDuringFastOpenPending;
+    bool                           mReceivedSocketWouldBlockDuringFastOpen;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsHttpConnection, NS_HTTPCONNECTION_IID)
 
 } // namespace net
 } // namespace mozilla
 
 #endif // nsHttpConnection_h__