Bug 1359938 - Fix socket status events for TCP Fast Open. r=mcmanus
authorDragana Damjanovic <dd.mozilla@gmail.com>
Thu, 04 May 2017 12:15:16 +0200
changeset 356639 4a3896a7e29f605d0bc509ca98095c1960aa70cf
parent 356638 e44d09824d9e81e409e8baaec88007fb9e85f0c0
child 356640 caf4f16f04370968e7219dc885e0578589f8dcf2
push id31768
push usercbook@mozilla.com
push dateFri, 05 May 2017 13:17:50 +0000
treeherdermozilla-central@9348b76977e8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus
bugs1359938
milestone55.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 1359938 - Fix socket status events for TCP Fast Open. r=mcmanus
netwerk/base/TCPFastOpenLayer.cpp
netwerk/base/TCPFastOpenLayer.h
netwerk/base/nsSocketTransport2.cpp
netwerk/base/nsSocketTransport2.h
netwerk/protocol/http/nsHttpTransaction.cpp
--- a/netwerk/base/TCPFastOpenLayer.cpp
+++ b/netwerk/base/TCPFastOpenLayer.cpp
@@ -52,27 +52,30 @@ static PRIOMethods   *sTCPFastOpenLayerM
  **/
 
 class TCPFastOpenSecret
 {
 public:
   TCPFastOpenSecret()
     : mState(WAITING_FOR_CONNECT)
     , mFirstPacketBufLen(0)
+    , mCondition(0)
   {}
 
   enum {
     CONNECTED,
     WAITING_FOR_CONNECTCONTINUE,
     COLLECT_DATA_FOR_FIRST_PACKET,
-    WAITING_FOR_CONNECT
+    WAITING_FOR_CONNECT,
+    SOCKET_ERROR_STATE
   } mState;
   PRNetAddr mAddr;
   char mFirstPacketBuf[1460];
   uint16_t mFirstPacketBufLen;
+  PRErrorCode mCondition;
 };
 
 static PRStatus
 TCPFastOpenConnect(PRFileDesc *fd, const PRNetAddr *addr,
                    PRIntervalTime timeout)
 {
   MOZ_RELEASE_ASSERT(fd->identity == sTCPFastOpenLayerIdentity);
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
@@ -156,16 +159,19 @@ TCPFastOpenSend(PRFileDesc *fd, const vo
       toSend = (toSend > amount) ? amount : toSend;
       memcpy(secret->mFirstPacketBuf + secret->mFirstPacketBufLen, buf, toSend);
       secret->mFirstPacketBufLen += toSend;
       return toSend;
     }
   case TCPFastOpenSecret::WAITING_FOR_CONNECT:
     PR_SetError(PR_NOT_CONNECTED_ERROR, 0);
     return -1;
+  case TCPFastOpenSecret::SOCKET_ERROR_STATE:
+    PR_SetError(secret->mCondition, 0);
+    return -1;
   }
   PR_SetError(PR_WOULD_BLOCK_ERROR, 0);
   return PR_FAILURE;
 }
 
 static PRInt32
 TCPFastOpenWrite(PRFileDesc *fd, const void *buf, PRInt32 amount)
 {
@@ -210,16 +216,19 @@ TCPFastOpenRecv(PRFileDesc *fd, void *bu
     rv = (fd->lower->methods->recv)(fd->lower, buf, amount, flags, timeout);
     break;
   case TCPFastOpenSecret::WAITING_FOR_CONNECTCONTINUE:
   case TCPFastOpenSecret::COLLECT_DATA_FOR_FIRST_PACKET:
     PR_SetError(PR_WOULD_BLOCK_ERROR, 0);
     break;
   case TCPFastOpenSecret::WAITING_FOR_CONNECT:
     PR_SetError(PR_NOT_CONNECTED_ERROR, 0);
+    break;
+  case TCPFastOpenSecret::SOCKET_ERROR_STATE:
+    PR_SetError(secret->mCondition, 0);
   }
   return rv;
 }
 
 static PRInt32
 TCPFastOpenRead(PRFileDesc *fd, void *buf, PRInt32 amount)
 {
   return TCPFastOpenRecv(fd, buf, amount , 0, PR_INTERVAL_NO_WAIT);
@@ -242,16 +251,20 @@ TCPFastOpenConnectContinue(PRFileDesc *f
     PR_SetError(PR_NOT_CONNECTED_ERROR, 0);
     rv = PR_FAILURE;
     break;
   case TCPFastOpenSecret::WAITING_FOR_CONNECTCONTINUE:
     rv = (fd->lower->methods->connectcontinue)(fd->lower, out_flags);
 
     SOCKET_LOG(("TCPFastOpenConnectContinue result=%d.\n", rv));
     secret->mState = TCPFastOpenSecret::CONNECTED;
+    break;
+  case TCPFastOpenSecret::SOCKET_ERROR_STATE:
+    PR_SetError(secret->mCondition, 0);
+    rv = PR_FAILURE;
   }
   return rv;
 }
 
 static PRStatus
 TCPFastOpenClose(PRFileDesc *fd)
 {
   if (!fd) {
@@ -466,10 +479,64 @@ TCPFastOpenGetBufferSizeLeft(PRFileDesc 
   sizeLeft -= secret->mFirstPacketBufLen;
 
   SOCKET_LOG(("TCPFastOpenGetBufferSizeLeft=%d.\n", sizeLeft));
 
   return (sizeLeft > TFO_TLS_RECORD_HEADER_SIZE) ?
     sizeLeft - TFO_TLS_RECORD_HEADER_SIZE : 0;
 }
 
+bool
+TCPFastOpenGetCurrentBufferSize(PRFileDesc *fd)
+{
+  PRFileDesc *tfoFd = PR_GetIdentitiesLayer(fd, sTCPFastOpenLayerIdentity);
+  MOZ_RELEASE_ASSERT(tfoFd);
+  MOZ_ASSERT(OnSocketThread(), "not on socket thread");
+
+  TCPFastOpenSecret *secret = reinterpret_cast<TCPFastOpenSecret *>(tfoFd->secret);
+
+  return secret->mFirstPacketBufLen;
+}
+
+bool
+TCPFastOpenFlushBuffer(PRFileDesc *fd)
+{
+  PRFileDesc *tfoFd = PR_GetIdentitiesLayer(fd, sTCPFastOpenLayerIdentity);
+  MOZ_RELEASE_ASSERT(tfoFd);
+  MOZ_ASSERT(OnSocketThread(), "not on socket thread");
+
+  TCPFastOpenSecret *secret = reinterpret_cast<TCPFastOpenSecret *>(tfoFd->secret);
+  MOZ_ASSERT(secret->mState == TCPFastOpenSecret::CONNECTED);
+
+  if (secret->mFirstPacketBufLen) {
+    SOCKET_LOG(("TCPFastOpenFlushBuffer - %d bytes to drain from "
+                "mFirstPacketBufLen.\n",
+                secret->mFirstPacketBufLen ));
+    PRInt32 rv = (tfoFd->lower->methods->send)(tfoFd->lower,
+                                               secret->mFirstPacketBuf,
+                                               secret->mFirstPacketBufLen,
+                                               0, // flags
+                                               PR_INTERVAL_NO_WAIT);
+    if (rv <= 0) {
+      PRErrorCode err = PR_GetError();
+      if (err == PR_WOULD_BLOCK_ERROR) {
+        // We still need to send this data.
+        return true;
+      } else {
+        // There is an error, let nsSocketTransport pick it up properly.
+        secret->mCondition = err;
+        secret->mState = TCPFastOpenSecret::SOCKET_ERROR_STATE;
+        return false;
+      }
+    }
+
+    secret->mFirstPacketBufLen -= rv;
+    if (secret->mFirstPacketBufLen) {
+      memmove(secret->mFirstPacketBuf,
+              secret->mFirstPacketBuf + rv,
+              secret->mFirstPacketBufLen);
+    }
+  }
+  return secret->mFirstPacketBufLen;
+}
+
 }
 }
--- a/netwerk/base/TCPFastOpenLayer.h
+++ b/netwerk/base/TCPFastOpenLayer.h
@@ -26,12 +26,15 @@ namespace net {
 
 nsresult AttachTCPFastOpenIOLayer(PRFileDesc *fd);
 
 // Get the result of TCP Fast Open.
 void TCPFastOpenFinish(PRFileDesc *fd, PRErrorCode &err,
                        bool &fastOpenNotSupported, uint8_t &tfoStatus);
 
 int32_t TCPFastOpenGetBufferSizeLeft(PRFileDesc *fd);
+
+bool TCPFastOpenGetCurrentBufferSize(PRFileDesc *fd);
+bool TCPFastOpenFlushBuffer(PRFileDesc *fd);
 }
 }
 
 #endif // TCPFastOpenLayer_h__
--- a/netwerk/base/nsSocketTransport2.cpp
+++ b/netwerk/base/nsSocketTransport2.cpp
@@ -595,25 +595,28 @@ nsSocketOutputStream::Write(const char *
     SOCKET_LOG(("nsSocketOutputStream::Write [this=%p count=%u]\n", this, count));
 
     *countWritten = 0;
 
     // A write of 0 bytes can be used to force the initial SSL handshake, so do
     // not reject that.
 
     PRFileDesc* fd = nullptr;
+    bool fastOpenInProgress;
     {
         MutexAutoLock lock(mTransport->mLock);
 
         if (NS_FAILED(mCondition))
             return mCondition;
         
         fd = mTransport->GetFD_LockedAlsoDuringFastOpen();
         if (!fd)
             return NS_BASE_STREAM_WOULD_BLOCK;
+
+        fastOpenInProgress = mTransport->FastOpenInProgress();
     }
 
     SOCKET_LOG(("  calling PR_Write [count=%u]\n", count));
 
     // cannot hold lock while calling NSPR.  (worried about the fact that PSM
     // synchronously proxies notifications over to the UI thread, which could
     // mistakenly try to re-enter this code.)
     int32_t n = PR_Write(fd, buf, count);
@@ -641,18 +644,24 @@ nsSocketOutputStream::Write(const char *
         }
         rv = mCondition;
     }
     if (NS_FAILED(rv))
         mTransport->OnOutputClosed(rv);
 
     // only send this notification if we have indeed written some data.
     // see bug 196827 for an example of why this is important.
-    if (n > 0)
+    // During a fast open we are actually not sending data, the data will be
+    // only buffered in the TCPFastOpenLayer. Therefore we will call
+    // SendStatus(NS_NET_STATUS_SENDING_TO) when we really send data (i.e. when
+    // TCPFastOpenFinish is called.
+    if ((n > 0) && !fastOpenInProgress) {
         mTransport->SendStatus(NS_NET_STATUS_SENDING_TO);
+    }
+
     return rv;
 }
 
 NS_IMETHODIMP
 nsSocketOutputStream::WriteSegments(nsReadSegmentFun reader, void *closure,
                                     uint32_t count, uint32_t *countRead)
 {
     // socket stream is unbuffered
@@ -764,16 +773,17 @@ nsSocketTransport::nsSocketTransport()
     , mInput(this)
     , mOutput(this)
     , mQoSBits(0x00)
     , mKeepaliveEnabled(false)
     , mKeepaliveIdleTimeS(-1)
     , mKeepaliveRetryIntervalS(-1)
     , mKeepaliveProbeCount(-1)
     , mFastOpenCallback(nullptr)
+    , mFastOpenLayerHasBufferedData(false)
 {
     SOCKET_LOG(("creating nsSocketTransport @%p\n", this));
 
     mTimeouts[TIMEOUT_CONNECT]    = UINT16_MAX; // no timeout
     mTimeouts[TIMEOUT_READ_WRITE] = UINT16_MAX; // no timeout
 }
 
 nsSocketTransport::~nsSocketTransport()
@@ -1519,16 +1529,28 @@ nsSocketTransport::InitiateSocket()
         }
         SOCKET_LOG(("Using TCP Fast Open."));
         rv = mFastOpenCallback->StartFastOpen(fd);
         status = PR_FAILURE;
         connectCalled = false;
         bool fastOpenNotSupported = false;
         uint8_t tfoStatus = TFO_NOT_TRIED;
         TCPFastOpenFinish(fd, code, fastOpenNotSupported, tfoStatus);
+
+        // If we have sent data, trigger a socket status event.
+        if (tfoStatus == TFO_DATA_SENT) {
+            SendStatus(NS_NET_STATUS_SENDING_TO);
+        }
+
+        // If we have still some data buffered this data must be flush before
+        // mOutput.OnSocketReady(NS_OK) is called in
+        // nsSocketTransport::OnSocketReady, partially to keep socket status
+        // event in order.
+        mFastOpenLayerHasBufferedData = TCPFastOpenGetCurrentBufferSize(fd);
+
         mFastOpenCallback->SetFastOpenStatus(tfoStatus);
         SOCKET_LOG(("called StartFastOpen - code=%d; fastOpen is %s "
                     "supported.\n", code,
                     fastOpenNotSupported ? "not" : ""));
         SOCKET_LOG(("TFO status %d\n", tfoStatus));
 
         if (fastOpenNotSupported) {
           // When TCP_FastOpen is turned off on the local host
@@ -1883,16 +1905,23 @@ nsSocketTransport::GetFD_LockedAlsoDurin
 
     if (mFD.IsInitialized()) {
         mFDref++;
     }
 
     return mFD;
 }
 
+bool
+nsSocketTransport::FastOpenInProgress()
+{
+    mLock.AssertCurrentThreadOwns();
+    return mFDFastOpenInProgress;
+}
+
 class ThunkPRClose : public Runnable
 {
 public:
   explicit ThunkPRClose(PRFileDesc *fd) : mFD(fd) {}
 
   NS_IMETHOD Run() override
   {
     nsSocketTransport::CloseSocket(mFD,
@@ -2065,16 +2094,33 @@ nsSocketTransport::OnSocketReady(PRFileD
         this, outFlags));
 
     if (outFlags == -1) {
         SOCKET_LOG(("socket timeout expired\n"));
         mCondition = NS_ERROR_NET_TIMEOUT;
         return;
     }
 
+    if ((mState == STATE_TRANSFERRING) && mFastOpenLayerHasBufferedData) {
+        // We have some data buffered in TCPFastOpenLayer. We will flush them
+        // first. We need to do this first before calling OnSocketReady below
+        // so that the socket status events are kept in the correct order.
+        mFastOpenLayerHasBufferedData = TCPFastOpenFlushBuffer(fd);
+        if (mFastOpenLayerHasBufferedData) {
+            return;
+        } else {
+            SendStatus(NS_NET_STATUS_SENDING_TO);
+        }
+        // If we are done sending the buffered data continue with the normal
+        // path.
+        // In case of an error, TCPFastOpenFlushBuffer will return false and
+        // the normal code path will pick up the error.
+        mFastOpenLayerHasBufferedData = false;
+    }
+
     if (mState == STATE_TRANSFERRING) {
         // if waiting to write and socket is writable or hit an exception.
         if ((mPollFlags & PR_POLL_WRITE) && (outFlags & ~PR_POLL_READ)) {
             // assume that we won't need to poll any longer (the stream will
             // request that we poll again if it is still pending).
             mPollFlags &= ~PR_POLL_WRITE;
             mOutput.OnSocketReady(NS_OK);
         }
@@ -2196,16 +2242,18 @@ nsSocketTransport::OnSocketDetached(PRFi
         if (gIOService->IsOffline()) {
           mCondition = NS_ERROR_OFFLINE;
         }
         else {
           mCondition = NS_ERROR_ABORT;
         }
     }
 
+    mFastOpenLayerHasBufferedData = false;
+
     // If we are not shutting down try again.
     if (!gIOService->IsNetTearingDown() && RecoverFromError())
         mCondition = NS_OK;
     else {
         mState = STATE_CLOSED;
 
         // make sure there isn't any pending DNS request
         if (mDNSRequest) {
--- a/netwerk/base/nsSocketTransport2.h
+++ b/netwerk/base/nsSocketTransport2.h
@@ -411,16 +411,17 @@ private:
     uint8_t mQoSBits;
 
     //
     // mFD access methods: called with mLock held.
     //
     PRFileDesc *GetFD_Locked();
     PRFileDesc *GetFD_LockedAlsoDuringFastOpen();
     void        ReleaseFD_Locked(PRFileDesc *fd);
+    bool FastOpenInProgress();
 
     //
     // stream state changes (called outside mLock):
     //
     void OnInputClosed(nsresult reason)
     {
         // no need to post an event if called on the socket thread
         if (OnSocketThread())
@@ -470,14 +471,15 @@ private:
 
     // Keepalive config (support varies by platform).
     int32_t mKeepaliveIdleTimeS;
     int32_t mKeepaliveRetryIntervalS;
     int32_t mKeepaliveProbeCount;
 
     // A Fast Open callback.
     TCPFastOpen *mFastOpenCallback;
+    bool mFastOpenLayerHasBufferedData;
 };
 
 } // namespace net
 } // namespace mozilla
 
 #endif // !nsSocketTransport_h__
--- a/netwerk/protocol/http/nsHttpTransaction.cpp
+++ b/netwerk/protocol/http/nsHttpTransaction.cpp
@@ -543,16 +543,19 @@ nsHttpTransaction::OnTransportStatus(nsI
         } else if (status == NS_NET_STATUS_RESOLVED_HOST) {
             SetDomainLookupEnd(TimeStamp::Now());
         } else if (status == NS_NET_STATUS_CONNECTING_TO) {
             SetConnectStart(TimeStamp::Now());
         } else if (status == NS_NET_STATUS_CONNECTED_TO) {
             SetConnectEnd(TimeStamp::Now(), true);
         } else if (status == NS_NET_STATUS_TLS_HANDSHAKE_ENDED) {
             SetConnectEnd(TimeStamp::Now(), false);
+        } else if (status == NS_NET_STATUS_SENDING_TO) {
+            // Set the timestamp to Now(), only if it null
+            SetRequestStart(TimeStamp::Now(), true);
         }
     }
 
     if (!mTransportSink)
         return;
 
     MOZ_ASSERT(OnSocketThread(), "not on socket thread");
 
@@ -675,21 +678,16 @@ nsHttpTransaction::ReadRequestSegment(ns
 {
     // For the tracking of sent bytes that we used to do for the networkstats
     // API, please see bug 1318883 where it was removed.
 
     nsHttpTransaction *trans = (nsHttpTransaction *) closure;
     nsresult rv = trans->mReader->OnReadSegment(buf, count, countRead);
     if (NS_FAILED(rv)) return rv;
 
-    if (trans->TimingEnabled()) {
-        // Set the timestamp to Now(), only if it null
-        trans->SetRequestStart(TimeStamp::Now(), true);
-    }
-
     trans->mSentData = true;
     return NS_OK;
 }
 
 nsresult
 nsHttpTransaction::ReadSegments(nsAHttpSegmentReader *reader,
                                 uint32_t count, uint32_t *countRead)
 {