bug 1132212 - nshttptransction.cpp dont play with data race fire r=sworkman
authorPatrick McManus <mcmanus@ducksong.com>
Thu, 12 Feb 2015 15:00:18 -0500
changeset 229093 216e9a560a661342cc5de882df0ea82a4548761e
parent 229092 c899f3f1fb08e078c19e8d18aeb7f899a9bed639
child 229094 ded390f05c90f01a05f8f924eaec5d24f530c42a
push id55590
push usermcmanus@ducksong.com
push dateSat, 14 Feb 2015 00:25:51 +0000
treeherdermozilla-inbound@216e9a560a66 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssworkman
bugs1132212
milestone38.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 1132212 - nshttptransction.cpp dont play with data race fire r=sworkman
netwerk/protocol/http/NullHttpTransaction.h
netwerk/protocol/http/nsHttpTransaction.cpp
netwerk/protocol/http/nsHttpTransaction.h
--- a/netwerk/protocol/http/NullHttpTransaction.h
+++ b/netwerk/protocol/http/NullHttpTransaction.h
@@ -54,20 +54,22 @@ protected:
 
 private:
   nsresult mStatus;
 protected:
   uint32_t mCaps;
 private:
   // mCapsToClear holds flags that should be cleared in mCaps, e.g. unset
   // NS_HTTP_REFRESH_DNS when DNS refresh request has completed to avoid
-  // redundant requests on the network. To deal with raciness, only unsetting
+  // redundant requests on the network. The member itself is atomic, but
+  // access to it from the networking thread may happen either before or
+  // after the main thread modifies it. To deal with raciness, only unsetting
   // bitfields should be allowed: 'lost races' will thus err on the
   // conservative side, e.g. by going ahead with a 2nd DNS refresh.
-  uint32_t mCapsToClear;
+  Atomic<uint32_t> mCapsToClear;
   nsHttpRequestHead *mRequestHead;
   bool mIsDone;
   bool mClaimed;
 
 protected:
   nsRefPtr<nsAHttpConnection> mConnection;
   nsCOMPtr<nsIInterfaceRequestor> mCallbacks;
   nsRefPtr<nsHttpConnectionInfo> mConnectionInfo;
--- a/netwerk/protocol/http/nsHttpTransaction.cpp
+++ b/netwerk/protocol/http/nsHttpTransaction.cpp
@@ -97,19 +97,19 @@ nsHttpTransaction::nsHttpTransaction()
     , mContentRead(0)
     , mInvalidResponseBytesRead(0)
     , mPushedStream(nullptr)
     , mChunkedDecoder(nullptr)
     , mStatus(NS_OK)
     , mPriority(0)
     , mRestartCount(0)
     , mCaps(0)
-    , mCapsToClear(0)
     , mClassification(CLASS_GENERAL)
     , mPipelinePosition(0)
+    , mCapsToClear(0)
     , mHttpVersion(NS_HTTP_VERSION_UNKNOWN)
     , mClosed(false)
     , mConnected(false)
     , mHaveStatusLine(false)
     , mHaveAllHeaders(false)
     , mTransactionDone(false)
     , mResponseIsComplete(false)
     , mDidContentStart(false)
--- a/netwerk/protocol/http/nsHttpTransaction.h
+++ b/netwerk/protocol/http/nsHttpTransaction.h
@@ -243,26 +243,29 @@ private:
     TimingStruct                    mTimings;
 
     nsresult                        mStatus;
 
     int16_t                         mPriority;
 
     uint16_t                        mRestartCount;        // the number of times this transaction has been restarted
     uint32_t                        mCaps;
-    // mCapsToClear holds flags that should be cleared in mCaps, e.g. unset
-    // NS_HTTP_REFRESH_DNS when DNS refresh request has completed to avoid
-    // redundant requests on the network. To deal with raciness, only unsetting
-    // bitfields should be allowed: 'lost races' will thus err on the
-    // conservative side, e.g. by going ahead with a 2nd DNS refresh.
-    uint32_t                        mCapsToClear;
     enum Classifier                 mClassification;
     int32_t                         mPipelinePosition;
     int64_t                         mMaxPipelineObjectSize;
 
+    // mCapsToClear holds flags that should be cleared in mCaps, e.g. unset
+    // NS_HTTP_REFRESH_DNS when DNS refresh request has completed to avoid
+    // redundant requests on the network. The member itself is atomic, but
+    // access to it from the networking thread may happen either before or
+    // after the main thread modifies it. To deal with raciness, only unsetting
+    // bitfields should be allowed: 'lost races' will thus err on the
+    // conservative side, e.g. by going ahead with a 2nd DNS refresh.
+    Atomic<uint32_t>                mCapsToClear;
+
     nsHttpVersion                   mHttpVersion;
 
     // state flags, all logically boolean, but not packed together into a
     // bitfield so as to avoid bitfield-induced races.  See bug 560579.
     bool                            mClosed;
     bool                            mConnected;
     bool                            mHaveStatusLine;
     bool                            mHaveAllHeaders;