Bug 1159280 - TSan: data race netwerk/protocol/websocket/WebSocketChannel.cpp:3156 WebSocketChannel::Close, r=mcmanus
authorMichal Novotny <michal.novotny@gmail.com>
Sat, 23 May 2015 10:07:01 +0200
changeset 264917 0be288d81a00557f7149c6bdd67450fe5c791750
parent 264916 8eb4eb3288490a1ff402c12e238c035ce727cd10
child 264918 3cb74142cd09fdb9b12d2a8bfb02be2ae0346f27
push id8157
push userjlund@mozilla.com
push dateMon, 29 Jun 2015 20:36:23 +0000
treeherdermozilla-aurora@d480e05bd276 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus
bugs1159280
milestone41.0a1
Bug 1159280 - TSan: data race netwerk/protocol/websocket/WebSocketChannel.cpp:3156 WebSocketChannel::Close, r=mcmanus
netwerk/protocol/websocket/BaseWebSocketChannel.cpp
netwerk/protocol/websocket/BaseWebSocketChannel.h
netwerk/protocol/websocket/WebSocketChannel.cpp
netwerk/protocol/websocket/WebSocketChannel.h
--- a/netwerk/protocol/websocket/BaseWebSocketChannel.cpp
+++ b/netwerk/protocol/websocket/BaseWebSocketChannel.cpp
@@ -14,20 +14,20 @@
 #include "nsStandardURL.h"
 
 PRLogModuleInfo *webSocketLog = nullptr;
 
 namespace mozilla {
 namespace net {
 
 BaseWebSocketChannel::BaseWebSocketChannel()
-  : mEncrypted(0)
-  , mWasOpened(0)
+  : mWasOpened(0)
   , mClientSetPingInterval(0)
   , mClientSetPingTimeout(0)
+  , mEncrypted(0)
   , mPingForced(0)
   , mPingInterval(0)
   , mPingResponseTimeout(10000)
 {
   if (!webSocketLog)
     webSocketLog = PR_NewLogModule("nsWebSocket");
 }
 
@@ -140,16 +140,18 @@ BaseWebSocketChannel::GetPingInterval(ui
 
   *aSeconds = mPingInterval / 1000;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BaseWebSocketChannel::SetPingInterval(uint32_t aSeconds)
 {
+  MOZ_ASSERT(NS_IsMainThread());
+
   if (mWasOpened) {
     return NS_ERROR_IN_PROGRESS;
   }
 
   mPingInterval = aSeconds * 1000;
   mClientSetPingInterval = 1;
 
   return NS_OK;
@@ -163,16 +165,18 @@ BaseWebSocketChannel::GetPingTimeout(uin
 
   *aSeconds = mPingResponseTimeout / 1000;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BaseWebSocketChannel::SetPingTimeout(uint32_t aSeconds)
 {
+  MOZ_ASSERT(NS_IsMainThread());
+
   if (mWasOpened) {
     return NS_ERROR_IN_PROGRESS;
   }
 
   mPingResponseTimeout = aSeconds * 1000;
   mClientSetPingTimeout = 1;
 
   return NS_OK;
--- a/netwerk/protocol/websocket/BaseWebSocketChannel.h
+++ b/netwerk/protocol/websocket/BaseWebSocketChannel.h
@@ -84,21 +84,22 @@ class BaseWebSocketChannel : public nsIW
   nsCOMPtr<nsILoadInfo>           mLoadInfo;
   nsCOMPtr<nsIEventTarget>        mTargetThread;
 
   nsCString                       mProtocol;
   nsCString                       mOrigin;
 
   nsCString                       mNegotiatedExtensions;
 
-  uint32_t                        mEncrypted                 : 1;
   uint32_t                        mWasOpened                 : 1;
   uint32_t                        mClientSetPingInterval     : 1;
   uint32_t                        mClientSetPingTimeout      : 1;
-  uint32_t                        mPingForced                : 1;
+
+  Atomic<bool>                    mEncrypted;
+  bool                            mPingForced;
 
   uint32_t                        mPingInterval;         /* milliseconds */
   uint32_t                        mPingResponseTimeout;  /* milliseconds */
 };
 
 } // namespace net
 } // namespace mozilla
 
--- a/netwerk/protocol/websocket/WebSocketChannel.cpp
+++ b/netwerk/protocol/websocket/WebSocketChannel.cpp
@@ -1109,30 +1109,30 @@ uint32_t WebSocketChannel::sSerialSeed =
 WebSocketChannel::WebSocketChannel() :
   mPort(0),
   mCloseTimeout(20000),
   mOpenTimeout(20000),
   mConnecting(NOT_CONNECTING),
   mMaxConcurrentConnections(200),
   mGotUpgradeOK(0),
   mRecvdHttpUpgradeTransport(0),
+  mAutoFollowRedirects(0),
+  mAllowPMCE(1),
+  mPingOutstanding(0),
+  mReleaseOnTransmit(0),
+  mDataStarted(0),
   mRequestedClose(0),
   mClientClosed(0),
   mServerClosed(0),
   mStopped(0),
   mCalledOnStop(0),
-  mPingOutstanding(0),
-  mAutoFollowRedirects(0),
-  mReleaseOnTransmit(0),
   mTCPClosed(0),
   mOpenedHttpChannel(0),
-  mDataStarted(0),
   mIncrementedSessionCount(0),
   mDecrementedSessionCount(0),
-  mAllowPMCE(1),
   mMaxMessageSize(INT32_MAX),
   mStopOnClose(NS_OK),
   mServerCloseCode(CLOSE_ABNORMAL),
   mScriptCloseCode(0),
   mFragmentOpcode(kContinuation),
   mFragmentAccumulator(0),
   mBuffered(0),
   mBufferSize(kIncomingBufferInitialSize),
@@ -2223,16 +2223,23 @@ WebSocketChannel::CleanupConnection()
 
 void
 WebSocketChannel::StopSession(nsresult reason)
 {
   LOG(("WebSocketChannel::StopSession() %p [%x]\n", this, reason));
 
   // normally this should be called on socket thread, but it is ok to call it
   // from OnStartRequest before the socket thread machine has gotten underway
+  if (NS_IsMainThread()) {
+    MOZ_DIAGNOSTIC_ASSERT(!mDataStarted);
+  } else {
+    MOZ_DIAGNOSTIC_ASSERT(PR_GetCurrentThread() == gSocketThread,
+                          "Called on unexpected thread!");
+    MOZ_DIAGNOSTIC_ASSERT(mDataStarted);
+  }
 
   mStopped = 1;
 
   if (!mOpenedHttpChannel) {
     // The HTTP channel information will never be used in this case
     mChannel = nullptr;
     mHttpChannel = nullptr;
     mLoadGroup = nullptr;
@@ -2325,20 +2332,27 @@ WebSocketChannel::StopSession(nsresult r
     mTargetThread->Dispatch(runnable, NS_DISPATCH_NORMAL);
   }
 }
 
 void
 WebSocketChannel::AbortSession(nsresult reason)
 {
   LOG(("WebSocketChannel::AbortSession() %p [reason %x] stopped = %d\n",
-       this, reason, mStopped));
+       this, reason, !!mStopped));
 
   // normally this should be called on socket thread, but it is ok to call it
   // from the main thread before StartWebsocketData() has completed
+  if (NS_IsMainThread()) {
+    MOZ_DIAGNOSTIC_ASSERT(!mDataStarted);
+  } else {
+    MOZ_DIAGNOSTIC_ASSERT(PR_GetCurrentThread() == gSocketThread,
+                          "Called on unexpected thread!");
+    MOZ_DIAGNOSTIC_ASSERT(mDataStarted);
+  }
 
   // When we are failing we need to close the TCP connection immediately
   // as per 7.1.1
   mTCPClosed = true;
 
   if (mLingeringCloseTimer) {
     MOZ_ASSERT(mStopped, "Lingering without Stop");
     LOG(("WebSocketChannel:: Cleanup connection based on TCP Close"));
@@ -2362,17 +2376,17 @@ WebSocketChannel::AbortSession(nsresult 
   }
 }
 
 // ReleaseSession is called on orderly shutdown
 void
 WebSocketChannel::ReleaseSession()
 {
   LOG(("WebSocketChannel::ReleaseSession() %p stopped = %d\n",
-       this, mStopped));
+       this, !!mStopped));
   MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread, "not socket thread");
 
   if (mStopped)
     return;
   StopSession(NS_OK);
 }
 
 void
@@ -3688,40 +3702,42 @@ nsresult
 WebSocketChannel::SaveNetworkStats(bool enforce)
 {
 #ifdef MOZ_WIDGET_GONK
   // Check if the active network and app id are valid.
   if(!mActiveNetwork || mAppId == NECKO_NO_APP_ID) {
     return NS_OK;
   }
 
-  if (mCountRecv <= 0 && mCountSent <= 0) {
+  uint64_t countRecv = 0;
+  uint64_t countSent = 0;
+
+  mCountRecv.exchange(countRecv);
+  mCountSent.exchange(countSent);
+
+  if (countRecv == 0 && countSent == 0) {
     // There is no traffic, no need to save.
     return NS_OK;
   }
 
   // If |enforce| is false, the traffic amount is saved
   // only when the total amount exceeds the predefined
   // threshold.
-  uint64_t totalBytes = mCountRecv + mCountSent;
+  uint64_t totalBytes = countRecv + countSent;
   if (!enforce && totalBytes < NETWORK_STATS_THRESHOLD) {
     return NS_OK;
   }
 
   // Create the event to save the network statistics.
   // the event is then dispathed to the main thread.
   nsRefPtr<nsRunnable> event =
     new SaveNetworkStatsEvent(mAppId, mIsInBrowser, mActiveNetwork,
-                              mCountRecv, mCountSent, false);
+                              countRecv, countSent, false);
   NS_DispatchToMainThread(event);
 
-  // Reset the counters after saving.
-  mCountSent = 0;
-  mCountRecv = 0;
-
   return NS_OK;
 #else
   return NS_ERROR_NOT_IMPLEMENTED;
 #endif
 }
 
 } // namespace mozilla::net
 } // namespace mozilla
--- a/netwerk/protocol/websocket/WebSocketChannel.h
+++ b/netwerk/protocol/websocket/WebSocketChannel.h
@@ -24,16 +24,17 @@
 #ifdef MOZ_WIDGET_GONK
 #include "nsINetworkManager.h"
 #include "nsProxyRelease.h"
 #endif
 
 #include "nsCOMPtr.h"
 #include "nsString.h"
 #include "nsDeque.h"
+#include "mozilla/Atomics.h"
 
 class nsIAsyncVerifyRedirectCallback;
 class nsIDashboardEventNotifier;
 class nsIEventTarget;
 class nsIHttpChannel;
 class nsIRandomGenerator;
 class nsISocketTransport;
 class nsIURI;
@@ -213,32 +214,38 @@ private:
   nsCOMPtr<nsITimer>              mPingTimer;
 
   nsCOMPtr<nsITimer>              mLingeringCloseTimer;
   const static int32_t            kLingeringCloseTimeout =   1000;
   const static int32_t            kLingeringCloseThreshold = 50;
 
   int32_t                         mMaxConcurrentConnections;
 
+  // following members are accessed only on the main thread
   uint32_t                        mGotUpgradeOK              : 1;
   uint32_t                        mRecvdHttpUpgradeTransport : 1;
-  uint32_t                        mRequestedClose            : 1;
-  uint32_t                        mClientClosed              : 1;
-  uint32_t                        mServerClosed              : 1;
-  uint32_t                        mStopped                   : 1;
-  uint32_t                        mCalledOnStop              : 1;
+  uint32_t                        mAutoFollowRedirects       : 1;
+  uint32_t                        mAllowPMCE                 : 1;
+  uint32_t                                                   : 0;
+
+  // following members are accessed only on the socket thread
   uint32_t                        mPingOutstanding           : 1;
-  uint32_t                        mAutoFollowRedirects       : 1;
   uint32_t                        mReleaseOnTransmit         : 1;
-  uint32_t                        mTCPClosed                 : 1;
-  uint32_t                        mOpenedHttpChannel         : 1;
-  uint32_t                        mDataStarted               : 1;
-  uint32_t                        mIncrementedSessionCount   : 1;
-  uint32_t                        mDecrementedSessionCount   : 1;
-  uint32_t                        mAllowPMCE                 : 1;
+  uint32_t                                                   : 0;
+
+  Atomic<bool>                    mDataStarted;
+  Atomic<bool>                    mRequestedClose;
+  Atomic<bool>                    mClientClosed;
+  Atomic<bool>                    mServerClosed;
+  Atomic<bool>                    mStopped;
+  Atomic<bool>                    mCalledOnStop;
+  Atomic<bool>                    mTCPClosed;
+  Atomic<bool>                    mOpenedHttpChannel;
+  Atomic<bool>                    mIncrementedSessionCount;
+  Atomic<bool>                    mDecrementedSessionCount;
 
   int32_t                         mMaxMessageSize;
   nsresult                        mStopOnClose;
   uint16_t                        mServerCloseCode;
   nsCString                       mServerCloseReason;
   uint16_t                        mScriptCloseCode;
   nsCString                       mScriptCloseReason;
 
@@ -273,18 +280,18 @@ private:
   bool                            mPrivateBrowsing;
 
   nsCOMPtr<nsIDashboardEventNotifier> mConnectionLogService;
   uint32_t mSerial;
   static uint32_t sSerialSeed;
 
 // These members are used for network per-app metering (bug 855949)
 // Currently, they are only available on gonk.
-  uint64_t                        mCountRecv;
-  uint64_t                        mCountSent;
+  Atomic<uint64_t, Relaxed>       mCountRecv;
+  Atomic<uint64_t, Relaxed>       mCountSent;
   uint32_t                        mAppId;
   bool                            mIsInBrowser;
 #ifdef MOZ_WIDGET_GONK
   nsMainThreadPtrHandle<nsINetworkInterface> mActiveNetwork;
 #endif
   nsresult                        SaveNetworkStats(bool);
   void                            CountRecvBytes(uint64_t recvBytes)
   {