Bug 1207753 - InputStreamPump thread-safety annotations r=necko-reviewers,dragana

Differential Revision: https://phabricator.services.mozilla.com/D141610

netwerk/base/nsInputStreamPump.cpp

@@ -154,7 +154,7 @@ NS_IMETHODIMP
 nsInputStreamPump::IsPending(bool* result) {
   RecursiveMutexAutoLock lock(mMutex);
 
-  *result = (mState != STATE_IDLE);
+  *result = (mState != STATE_IDLE && mState != STATE_DEAD);
   return NS_OK;
 }
 
@@ -200,7 +200,8 @@ nsInputStreamPump::Suspend() {
   RecursiveMutexAutoLock lock(mMutex);
 
   LOG(("nsInputStreamPump::Suspend [this=%p]\n", this));
-  NS_ENSURE_TRUE(mState != STATE_IDLE, NS_ERROR_UNEXPECTED);
+  NS_ENSURE_TRUE(mState != STATE_IDLE && mState != STATE_DEAD,
+                 NS_ERROR_UNEXPECTED);
   ++mSuspendCount;
   return NS_OK;
 }
@@ -211,12 +212,15 @@ nsInputStreamPump::Resume() {
 
   LOG(("nsInputStreamPump::Resume [this=%p]\n", this));
   NS_ENSURE_TRUE(mSuspendCount > 0, NS_ERROR_UNEXPECTED);
-  NS_ENSURE_TRUE(mState != STATE_IDLE, NS_ERROR_UNEXPECTED);
+  NS_ENSURE_TRUE(mState != STATE_IDLE && mState != STATE_DEAD,
+                 NS_ERROR_UNEXPECTED);
 
   // There is a brief in-between state when we null out mAsyncStream in
   // OnStateStop() before calling OnStopRequest, and only afterwards set
-  // STATE_IDLE, which we need to handle gracefully.
-  if (--mSuspendCount == 0 && mAsyncStream) EnsureWaiting();
+  // STATE_DEAD, which we need to handle gracefully.
+  if (--mSuspendCount == 0 && mAsyncStream) {
+    EnsureWaiting();
+  }
   return NS_OK;
 }
 
@@ -359,7 +363,7 @@ nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream* stream) {
       break;
     }
     mProcessingCallbacks = true;
-    if (mSuspendCount || mState == STATE_IDLE) {
+    if (mSuspendCount || mState == STATE_IDLE || mState == STATE_DEAD) {
       mWaitingForInputStreamReady = false;
       mProcessingCallbacks = false;
       break;
@@ -460,8 +464,10 @@ uint32_t nsInputStreamPump::OnStateStart() {
     // nsInputStreamPumps are needed (e.g. nsHttpChannel).
     RecursiveMutexAutoUnlock unlock(mMutex);
     // We're on the writing thread
+    PUSH_IGNORE_THREAD_SAFETY
     AssertOnThread();
     rv = mListener->OnStartRequest(this);
+    POP_THREAD_SAFETY
   }
 
   // an error returned from OnStartRequest should cause us to abort; however,
@@ -531,8 +537,10 @@ uint32_t nsInputStreamPump::OnStateTransfer() {
       // We're on the writing thread for mListener and mAsyncStream.
       // mStreamOffset is only touched in OnStateTransfer, and AsyncRead
       // shouldn't be called during OnDataAvailable()
+
       // We may be called on non-MainThread even if mOffMainThread is
       // false, due to RetargetDeliveryTo(), so don't use AssertOnThread()
+      PUSH_IGNORE_THREAD_SAFETY
       if (mTargetThread) {
         MOZ_ASSERT(mTargetThread->IsOnCurrentThread());
       } else {
@@ -540,6 +548,7 @@ uint32_t nsInputStreamPump::OnStateTransfer() {
       }
       rv = mListener->OnDataAvailable(this, mAsyncStream, mStreamOffset,
                                       odaAvail);
+      POP_THREAD_SAFETY
     }
 
     // don't enter this code if ODA failed or called Cancel
@@ -609,8 +618,8 @@ uint32_t nsInputStreamPump::OnStateStop() {
     nsresult rv = mLabeledMainThreadTarget->Dispatch(
         NewRunnableMethod("nsInputStreamPump::CallOnStateStop", this,
                           &nsInputStreamPump::CallOnStateStop));
-    NS_ENSURE_SUCCESS(rv, STATE_IDLE);
-    return STATE_IDLE;
+    NS_ENSURE_SUCCESS(rv, STATE_DEAD);
+    return STATE_DEAD;
   }
 
   AUTO_PROFILER_LABEL("nsInputStreamPump::OnStateStop", NETWORK);
@@ -625,7 +634,7 @@ uint32_t nsInputStreamPump::OnStateStop() {
   if (!mAsyncStream || !mListener) {
     MOZ_ASSERT(mAsyncStream, "null mAsyncStream: OnStateStop called twice?");
     MOZ_ASSERT(mListener, "null mListener: OnStateStop called twice?");
-    return STATE_IDLE;
+    return STATE_DEAD;
   }
 
   if (NS_FAILED(mStatus)) {
@@ -643,15 +652,17 @@ uint32_t nsInputStreamPump::OnStateStop() {
     RecursiveMutexAutoUnlock unlock(mMutex);
     // We're on the writing thread.
     // We believe that mStatus can't be changed on us here.
+    PUSH_IGNORE_THREAD_SAFETY
     AssertOnThread();
     mListener->OnStopRequest(this, mStatus);
+    POP_THREAD_SAFETY
   }
   mTargetThread = nullptr;
   mListener = nullptr;
 
   if (mLoadGroup) mLoadGroup->RemoveRequest(this, nullptr, mStatus);
 
-  return STATE_IDLE;
+  return STATE_DEAD;
 }
 
 nsresult nsInputStreamPump::CreateBufferedStreamIfNeeded() {

netwerk/base/nsInputStreamPump.h

@@ -71,56 +71,58 @@ class nsInputStreamPump final : public nsIInputStreamPump,
   nsresult CallOnStateStop();
 
  protected:
-  enum { STATE_IDLE, STATE_START, STATE_TRANSFER, STATE_STOP };
+  enum { STATE_IDLE, STATE_START, STATE_TRANSFER, STATE_STOP, STATE_DEAD };
 
   nsresult EnsureWaiting();
   uint32_t OnStateStart();
   uint32_t OnStateTransfer();
   uint32_t OnStateStop();
-  nsresult CreateBufferedStreamIfNeeded();
+  nsresult CreateBufferedStreamIfNeeded() REQUIRES(mMutex);
 
   // This should optimize away in non-DEBUG builds
   MOZ_ALWAYS_INLINE void AssertOnThread() const {
+    PUSH_IGNORE_THREAD_SAFETY
     if (mOffMainThread) {
       MOZ_ASSERT(mTargetThread->IsOnCurrentThread());
     } else {
       MOZ_ASSERT(NS_IsMainThread());
     }
+    POP_THREAD_SAFETY
   }
 
-  uint32_t mState{STATE_IDLE};
-  nsCOMPtr<nsILoadGroup> mLoadGroup;
-  nsCOMPtr<nsIStreamListener> mListener;
-  nsCOMPtr<nsIEventTarget> mTargetThread;
-  nsCOMPtr<nsIEventTarget> mLabeledMainThreadTarget;
-  nsCOMPtr<nsIInputStream> mStream;
-  nsCOMPtr<nsIAsyncInputStream> mAsyncStream;
-  uint64_t mStreamOffset{0};
-  uint64_t mStreamLength{0};
-  uint32_t mSegSize{0};
-  uint32_t mSegCount{0};
-  nsresult mStatus{NS_OK};
-  uint32_t mSuspendCount{0};
-  uint32_t mLoadFlags{LOAD_NORMAL};
-  bool mIsPending{false};
+  uint32_t mState GUARDED_BY(mMutex){STATE_IDLE};
+  nsCOMPtr<nsILoadGroup> mLoadGroup GUARDED_BY(mMutex);
   // mListener is written on a single thread (either MainThread or an
   // off-MainThread thread), read from that thread and perhaps others (in
   // RetargetDeliveryTo)
+  nsCOMPtr<nsIStreamListener> mListener GUARDED_BY(mMutex);
+  nsCOMPtr<nsIEventTarget> mTargetThread GUARDED_BY(mMutex);
+  nsCOMPtr<nsIEventTarget> mLabeledMainThreadTarget GUARDED_BY(mMutex);
+  nsCOMPtr<nsIInputStream> mStream GUARDED_BY(mMutex);
   // mAsyncStream is written on a single thread (either MainThread or an
   // off-MainThread thread), and lives from AsyncRead() to OnStateStop().
+  nsCOMPtr<nsIAsyncInputStream> mAsyncStream GUARDED_BY(mMutex);
+  uint64_t mStreamOffset GUARDED_BY(mMutex){0};
+  uint64_t mStreamLength GUARDED_BY(mMutex){0};
+  uint32_t mSegSize GUARDED_BY(mMutex){0};
+  uint32_t mSegCount GUARDED_BY(mMutex){0};
+  nsresult mStatus GUARDED_BY(mMutex){NS_OK};
+  uint32_t mSuspendCount GUARDED_BY(mMutex){0};
+  uint32_t mLoadFlags GUARDED_BY(mMutex){LOAD_NORMAL};
+  bool mIsPending GUARDED_BY(mMutex){false};
   // True while in OnInputStreamReady, calling OnStateStart, OnStateTransfer
   // and OnStateStop. Used to prevent calls to AsyncWait during callbacks.
-  bool mProcessingCallbacks{false};
+  bool mProcessingCallbacks GUARDED_BY(mMutex){false};
   // True if waiting on the "input stream ready" callback.
-  bool mWaitingForInputStreamReady{false};
-  bool mCloseWhenDone{false};
-  bool mRetargeting{false};
-  bool mAsyncStreamIsBuffered{false};
+  bool mWaitingForInputStreamReady GUARDED_BY(mMutex){false};
+  bool mCloseWhenDone GUARDED_BY(mMutex){false};
+  bool mRetargeting GUARDED_BY(mMutex){false};
+  bool mAsyncStreamIsBuffered GUARDED_BY(mMutex){false};
   // Indicate whether nsInputStreamPump is used completely off main thread.
-  bool mOffMainThread;
   // If true, OnStateStop() is executed off main thread. Set at creation.
+  const bool mOffMainThread;
   // Protects state/member var accesses across multiple threads.
-  mozilla::RecursiveMutex mMutex MOZ_UNANNOTATED{"nsInputStreamPump"};
+  mozilla::RecursiveMutex mMutex{"nsInputStreamPump"};
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsInputStreamPump, NS_INPUT_STREAM_PUMP_IID)