Issue #1695 - Fix socket timeout logic.

2 files changed, 121 insertions, 67 deletions

diff --git a/netwerk/base/nsSocketTransportService2.cpp b/netwerk/base/nsSocketTransportService2.cpp
index 4a8d80eed..6631db655 100644
--- a/netwerk/base/nsSocketTransportService2.cpp
+++ b/netwerk/base/nsSocketTransportService2.cpp
@@ -1,4 +1,3 @@
-// vim:set sw=4 sts=4 et cin:
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
@@ -53,6 +52,51 @@ uint32_t nsSocketTransportService::gMaxCount;
 PRCallOnceType nsSocketTransportService::gMaxCountInitOnce;
 
 //-----------------------------------------------------------------------------
+bool
+nsSocketTransportService::SocketContext::IsTimedOut(PRIntervalTime now) const
+{
+    return TimeoutIn(now) == 0;
+}
+
+void
+nsSocketTransportService::SocketContext::StartTimeout(PRIntervalTime now)
+{
+    if (!mPollStartEpoch) {
+        mPollStartEpoch = now;
+    }
+}
+
+void
+nsSocketTransportService::SocketContext::StopTimeout()
+{
+    mPollStartEpoch = 0;
+}
+
+void
+nsSocketTransportService::SocketContext::ResetTimeout()
+{
+  if (mPollStartEpoch && mHandler->mPollTimeout == UINT16_MAX) {
+    mPollStartEpoch = 0;
+  }
+}
+
+PRIntervalTime
+nsSocketTransportService::SocketContext::TimeoutIn(PRIntervalTime now) const
+{
+    if (mHandler->mPollTimeout == UINT16_MAX || !mPollStartEpoch) {
+        return NS_SOCKET_POLL_TIMEOUT;
+    }
+
+    PRIntervalTime elapsed = (now - mPollStartEpoch);
+    PRIntervalTime timeout = PR_SecondsToInterval(mHandler->mPollTimeout);
+
+    if (elapsed >= timeout) {
+        return 0;
+    }
+    return timeout - elapsed;
+}
+
+//-----------------------------------------------------------------------------
 // ctor/dtor (called on the main/UI thread by the service manager)
 
 nsSocketTransportService::nsSocketTransportService()
@@ -189,7 +233,7 @@ nsSocketTransportService::AttachSocket(PRFileDesc *fd, nsASocketHandler *handler
     SocketContext sock;
     sock.mFD = fd;
     sock.mHandler = handler;
-    sock.mElapsedTime = 0;
+    sock.mPollStartEpoch = 0;
 
     nsresult rv = AddToIdleList(&sock);
     if (NS_SUCCEEDED(rv))
@@ -275,6 +319,8 @@ nsSocketTransportService::AddToPollList(SocketContext *sock)
       PodMove(mPollList + newSocketIndex + 2, mPollList + newSocketIndex + 1,
               mActiveCount - newSocketIndex);
     }
+    
+    sock->StartTimeout(PR_IntervalNow());
     mActiveList[newSocketIndex] = *sock;
     mActiveCount++;
 
@@ -399,34 +445,32 @@ nsSocketTransportService::GrowIdleList()
 }
 
 PRIntervalTime
-nsSocketTransportService::PollTimeout()
+nsSocketTransportService::PollTimeout(PRIntervalTime now)
 {
-    if (mActiveCount == 0)
+    if (mActiveCount == 0) {
         return NS_SOCKET_POLL_TIMEOUT;
+    }
 
     // compute minimum time before any socket timeout expires.
-    uint32_t minR = UINT16_MAX;
+    PRIntervalTime minR = NS_SOCKET_POLL_TIMEOUT;
     for (uint32_t i=0; i<mActiveCount; ++i) {
         const SocketContext &s = mActiveList[i];
-        // mPollTimeout could be less than mElapsedTime if setTimeout
-        // was called with a value smaller than mElapsedTime.
-        uint32_t r = (s.mElapsedTime < s.mHandler->mPollTimeout)
-          ? s.mHandler->mPollTimeout - s.mElapsedTime
-          : 0;
-        if (r < minR)
+        PRIntervalTime r = s.TimeoutIn(now);
+        if (r < minR) {
             minR = r;
+        }
     }
-    // nsASocketHandler defines UINT16_MAX as do not timeout
-    if (minR == UINT16_MAX) {
+    
+    if (minR == NS_SOCKET_POLL_TIMEOUT) {
         SOCKET_LOG(("poll timeout: none\n"));
         return NS_SOCKET_POLL_TIMEOUT;
     }
-    SOCKET_LOG(("poll timeout: %lu\n", minR));
-    return PR_SecondsToInterval(minR);
+    SOCKET_LOG(("poll timeout: %lu seconds\n", PR_IntervalToSeconds(minR)));
+    return minR;
 }
 
 int32_t
-nsSocketTransportService::Poll(uint32_t *interval)
+nsSocketTransportService::Poll(PRIntervalTime ts)
 {
     PRPollDesc *pollList;
     uint32_t pollCount;
@@ -436,12 +480,12 @@ nsSocketTransportService::Poll(uint32_t *interval)
     // DoPollIteration() should service the network without blocking.
     bool pendingEvents = false;
     mRawThread->HasPendingEvents(&pendingEvents);
-
+    
     if (mPollList[0].fd) {
         mPollList[0].out_flags = 0;
         pollList = mPollList;
         pollCount = mActiveCount + 1;
-        pollTimeout = pendingEvents ? PR_INTERVAL_NO_WAIT : PollTimeout();
+        pollTimeout = pendingEvents ? PR_INTERVAL_NO_WAIT : PollTimeout(ts);
     }
     else {
         // no pollable event, so busy wait...
@@ -454,18 +498,14 @@ nsSocketTransportService::Poll(uint32_t *interval)
             pendingEvents ? PR_INTERVAL_NO_WAIT : PR_MillisecondsToInterval(25);
     }
 
-    PRIntervalTime ts = PR_IntervalNow();
-
     SOCKET_LOG(("    timeout = %i milliseconds\n",
          PR_IntervalToMilliseconds(pollTimeout)));
-    int32_t rv = PR_Poll(pollList, pollCount, pollTimeout);
 
-    PRIntervalTime passedInterval = PR_IntervalNow() - ts;
+    int32_t rv = PR_Poll(pollList, pollCount, pollTimeout);
 
     SOCKET_LOG(("    ...returned after %i milliseconds\n",
-         PR_IntervalToMilliseconds(passedInterval))); 
+         PR_IntervalToMilliseconds(PR_IntervalNow() - ts))); 
 
-    *interval = PR_IntervalToSeconds(passedInterval);
     return rv;
 }
 
@@ -811,13 +851,8 @@ nsSocketTransportService::Run()
     // make sure the pseudo random number generator is seeded on this thread
     srand(static_cast<unsigned>(PR_Now()));
 
-    int numberOfPendingEvents;
-
-    // If there is too many pending events queued, we will run some poll()
-    // between them.
     for (;;) {
         bool pendingEvents = false;
-        numberOfPendingEvents = 0;
 
         do {
             DoPollIteration();
@@ -838,7 +873,6 @@ nsSocketTransportService::Run()
                 TimeStamp eventQueueStart = TimeStamp::NowLoRes();
                 do {
                     NS_ProcessNextEvent(mRawThread);
-                    numberOfPendingEvents++;
                     pendingEvents = false;
                     mRawThread->HasPendingEvents(&pendingEvents);
                 } while (pendingEvents && mServingPendingQueue &&
@@ -915,6 +949,9 @@ nsSocketTransportService::DoPollIteration()
 {
     SOCKET_LOG(("STS poll iter\n"));
 
+    // Freeze "now" for list updates and polling.
+    PRIntervalTime now = PR_IntervalNow();
+    
     int32_t i, count;
     //
     // poll loop
@@ -932,16 +969,18 @@ nsSocketTransportService::DoPollIteration()
             mActiveList[i].mHandler->mCondition,
             mActiveList[i].mHandler->mPollFlags));
         //---
-        if (NS_FAILED(mActiveList[i].mHandler->mCondition))
+        if (NS_FAILED(mActiveList[i].mHandler->mCondition)) {
             DetachSocket(mActiveList, &mActiveList[i]);
-        else {
+        } else {
             uint16_t in_flags = mActiveList[i].mHandler->mPollFlags;
-            if (in_flags == 0)
+            if (in_flags == 0) {
                 MoveToIdleList(&mActiveList[i]);
-            else {
+            } else {
                 // update poll flags
                 mPollList[i+1].in_flags = in_flags;
                 mPollList[i+1].out_flags = 0;
+                // Active polling entry; start timeout.
+                mActiveList[i].StartTimeout(now);
             }
         }
     }
@@ -952,10 +991,11 @@ nsSocketTransportService::DoPollIteration()
             mIdleList[i].mHandler->mCondition,
             mIdleList[i].mHandler->mPollFlags));
         //---
-        if (NS_FAILED(mIdleList[i].mHandler->mCondition))
+        if (NS_FAILED(mIdleList[i].mHandler->mCondition)) {
             DetachSocket(mIdleList, &mIdleList[i]);
-        else if (mIdleList[i].mHandler->mPollFlags != 0)
+        } else if (mIdleList[i].mHandler->mPollFlags != 0) {
             MoveToPollList(&mIdleList[i]);
+        }
     }
 
     SOCKET_LOG(("  calling PR_Poll [active=%u idle=%u]\n", mActiveCount, mIdleCount));
@@ -969,55 +1009,44 @@ nsSocketTransportService::DoPollIteration()
 #endif
 
     // Measures seconds spent while blocked on PR_Poll
-    uint32_t pollInterval = 0;
     int32_t n = 0;
+    
     if (!gIOService->IsNetTearingDown()) {
         // Let's not do polling during shutdown.
 #if defined(XP_WIN)
         StartPolling();
 #endif
-        n = Poll(&pollInterval);
+        n = Poll(now);
 #if defined(XP_WIN)
         EndPolling();
 #endif
     }
+    
+    // Refresh when "now" is for following checks.
+    now = PR_IntervalNow();
 
     if (n < 0) {
         SOCKET_LOG(("  PR_Poll error [%d] os error [%d]\n", PR_GetError(),
                     PR_GetOSError()));
-    }
-    else {
+    } else {
         //
         // service "active" sockets...
         //
-        uint32_t numberOfOnSocketReadyCalls = 0;
         for (i=0; i<int32_t(mActiveCount); ++i) {
             PRPollDesc &desc = mPollList[i+1];
             SocketContext &s = mActiveList[i];
             if (n > 0 && desc.out_flags != 0) {
-                s.mElapsedTime = 0;
+                s.StopTimeout();
                 s.mHandler->OnSocketReady(desc.fd, desc.out_flags);
-                numberOfOnSocketReadyCalls++;
-            }
-            // check for timeout errors unless disabled...
-            else if (s.mHandler->mPollTimeout != UINT16_MAX) {
-                // update elapsed time counter
-                // (NOTE: We explicitly cast UINT16_MAX to be an unsigned value
-                // here -- otherwise, some compilers will treat it as signed,
-                // which makes them fire signed/unsigned-comparison build
-                // warnings for the comparison against 'pollInterval'.)
-                if (MOZ_UNLIKELY(pollInterval >
-                                static_cast<uint32_t>(UINT16_MAX) -
-                                s.mElapsedTime))
-                    s.mElapsedTime = UINT16_MAX;
-                else
-                    s.mElapsedTime += uint16_t(pollInterval);
-                // check for timeout expiration 
-                if (s.mElapsedTime >= s.mHandler->mPollTimeout) {
-                    s.mElapsedTime = 0;
-                    s.mHandler->OnSocketReady(desc.fd, -1);
-                    numberOfOnSocketReadyCalls++;
-                }
+            } else if (s.IsTimedOut(now)) {
+                // Socket timed out; disengage.
+                s.StopTimeout();
+                s.mHandler->OnSocketReady(desc.fd, -1);
+            } else {
+                // We may have recorded a timeout start on a socket and subsequently
+                // set it to not time out. Check the socket and reset the timestamp
+                // in this case to keep our states predictable.
+                s.ResetTimeout();
             }
         }
 
diff --git a/netwerk/base/nsSocketTransportService2.h b/netwerk/base/nsSocketTransportService2.h
index 0b88a6535..31efec1d9 100644
--- a/netwerk/base/nsSocketTransportService2.h
+++ b/netwerk/base/nsSocketTransportService2.h
@@ -19,7 +19,6 @@
 #include "mozilla/Mutex.h"
 #include "mozilla/net/DashboardTypes.h"
 #include "mozilla/Atomics.h"
-#include "mozilla/TimeStamp.h"
 #include "nsITimer.h"
 #include "mozilla/UniquePtr.h"
 #include "PollableEvent.h"
@@ -167,7 +166,33 @@ private:
     {
         PRFileDesc       *mFD;
         nsASocketHandler *mHandler;
-        uint16_t          mElapsedTime;  // time elapsed w/o activity
+        PRIntervalTime    mPollStartEpoch;  // Epoch timestamp when we started to poll this socket
+
+    public:
+        // Helper functions implementing a timeout mechanism.
+        
+        // Returns true if the socket has not been signalled in more than the desired
+        // timeout for this socket (mHandler->mPollTimeout).
+        bool IsTimedOut(PRIntervalTime now) const;
+        
+        // Records the epoch timestamp we started polling this socket. If the epoch is already
+        // recorded, then it does nothing (i.e. does not re-arm) so it's safe to call whenever
+        // this socket is put into the active polling list.
+        void StartTimeout(PRIntervalTime now);
+        
+        // Turns off the timout calculation.
+        void StopTimeout();
+        
+        // Returns the number of intervals from "now" after which this socket will timeout,
+        // or 0 (zero) when it has already timed out. Returns NS_SOCKET_POLL_TIMEOUT
+        // when there is no timeout set on the socket.
+        PRIntervalTime TimeoutIn(PRIntervalTime now) const;
+        
+        // When a socket timeout is set to not time out and later set again to time out, it
+        // is possible that mPollStartEpoch is not reset in-between. We have to manually
+        // call this on every iteration over sockets to ensure the epoch timestamp is reset
+        // and our socket bookkeeping remains accurate.
+        void ResetTimeout();
     };
 
     SocketContext *mActiveList;                   /* mListSize entries */
@@ -203,10 +228,10 @@ private:
 
     PRPollDesc *mPollList;                        /* mListSize + 1 entries */
 
-    PRIntervalTime PollTimeout();            // computes ideal poll timeout
+    PRIntervalTime PollTimeout(PRIntervalTime now); // computes ideal poll timeout
     nsresult       DoPollIteration();
                                              // perfoms a single poll iteration
-    int32_t        Poll(uint32_t *interval);
+    int32_t        Poll(PRIntervalTime now);
                                              // calls PR_Poll.  the out param
                                              // interval indicates the poll
                                              // duration in seconds.