Bug 913151 - Always call nsInputStreamPump::OnStateTransfer on the main thread r=jduell a=akeybl
authorSteve Workman <sworkman@mozilla.com>
Wed, 11 Sep 2013 12:55:15 -0700
changeset 213439 444b4437151cb17243259806d9c0161c243c8835
parent 213438 d476bbada7f51177e97668d84d942f5fe94203ee
child 213440 92fe3e4ad43eca2c7a4dfe3e27607c55216ec28d
push id3
push usergszorc@mozilla.com
push dateWed, 29 Oct 2014 02:45:36 +0000
reviewersjduell, akeybl
bugs913151
milestone25.0a2
Bug 913151 - Always call nsInputStreamPump::OnStateTransfer on the main thread r=jduell a=akeybl
netwerk/base/src/nsInputStreamPump.cpp
netwerk/base/src/nsInputStreamPump.h
--- a/netwerk/base/src/nsInputStreamPump.cpp
+++ b/netwerk/base/src/nsInputStreamPump.cpp
@@ -574,53 +574,54 @@ nsInputStreamPump::OnStateTransfer()
             if (rv != NS_BASE_STREAM_CLOSED)
                 mStatus = rv;
         }
     }
     return STATE_STOP;
 }
 
 nsresult
-nsInputStreamPump::OnStateStopForFailure()
+nsInputStreamPump::CallOnStateStop()
 {
-    MOZ_ASSERT(NS_FAILED(mStatus), "OnStateStopForFailure should be called "
-                                   "in a failed state");
-    MOZ_ASSERT(NS_IsMainThread(), "OnStateStopForFailure should be on the "
-                                  "main thread");
+    MOZ_ASSERT(NS_IsMainThread(),
+               "CallOnStateStop should only be called on the main thread.");
 
     mState = OnStateStop();
     return NS_OK;
 }
 
 uint32_t
 nsInputStreamPump::OnStateStop()
 {
-    if (NS_FAILED(mStatus)) {
-        // If EnsureWaiting has failed, it's possible that we could be off main
-        // thread. We may have to dispatch OnStateStop to the main thread
-        // directly. Note: this would result in OnStateStop being called
-        // outside the context of OnInputStreamReady.
-        if (!NS_IsMainThread()) {
-            nsresult rv = NS_DispatchToMainThread(
-                NS_NewRunnableMethod(this, &nsInputStreamPump::OnStateStopForFailure));
-            NS_ENSURE_SUCCESS(rv, STATE_IDLE);
-            return STATE_IDLE;
-        }
-    } else {
-        MOZ_ASSERT(NS_IsMainThread(), "In a success state, OnStateStop should "
-                                      "be on the main thread");
+    if (!NS_IsMainThread()) {
+        // Hopefully temporary hack: OnStateStop should only run on the main
+        // thread, but we're seeing some rare off-main-thread calls. For now
+        // just redispatch to the main thread in release builds, and crash in
+        // debug builds.
+        MOZ_ASSERT(NS_IsMainThread(),
+                   "OnStateStop should only be called on the main thread.");
+        nsresult rv = NS_DispatchToMainThread(
+            NS_NewRunnableMethod(this, &nsInputStreamPump::CallOnStateStop));
+        NS_ENSURE_SUCCESS(rv, STATE_IDLE);
+        return STATE_IDLE;
     }
 
     PROFILER_LABEL("Input", "nsInputStreamPump::OnStateTransfer");
     LOG(("  OnStateStop [this=%p status=%x]\n", this, mStatus));
 
     // if an error occurred, we must be sure to pass the error onto the async
     // stream.  in some cases, this is redundant, but since close is idempotent,
     // this is OK.  otherwise, be sure to honor the "close-when-done" option.
 
+    if (!mAsyncStream || !mListener) {
+        MOZ_ASSERT(mAsyncStream, "null mAsyncStream: OnStateStop called twice?");
+        MOZ_ASSERT(mListener, "null mListener: OnStateStop called twice?");
+        return STATE_IDLE;
+    }
+
     if (NS_FAILED(mStatus))
         mAsyncStream->CloseWithStatus(mStatus);
     else if (mCloseWhenDone)
         mAsyncStream->Close();
 
     mAsyncStream = 0;
     mTargetThread = 0;
     mIsPending = false;
--- a/netwerk/base/src/nsInputStreamPump.h
+++ b/netwerk/base/src/nsInputStreamPump.h
@@ -52,28 +52,20 @@ public:
      * The data from the stream will not be consumed, i.e. the pump's listener
      * can still read all the data.
      *
      * Do not call before asyncRead. Do not call after onStopRequest.
      */
     NS_HIDDEN_(nsresult) PeekStream(PeekSegmentFun callback, void *closure);
 
     /**
-     * Called on the main thread to clean up member variables. Called directly
-     * from OnStateStop if on the main thread, or dispatching to the main
-     * thread if not. Must be called on the main thread to serialize member
-     * variable deletion with calls to Cancel.
+     * Dispatched (to the main thread) by OnStateStop if it's called off main
+     * thread. Updates mState based on return value of OnStateStop.
      */
-    void OnStateStopCleanup();
-
-    /**
-     * Called on the main thread if EnsureWaiting fails, so that we always call
-     * OnStopRequest on main thread.
-     */
-    nsresult OnStateStopForFailure();
+    nsresult CallOnStateStop();
 
 protected:
 
     enum {
         STATE_IDLE,
         STATE_START,
         STATE_TRANSFER,
         STATE_STOP