From ad84723ad245642f71758e7607bb2aa4d7d42656 Mon Sep 17 00:00:00 2001 From: Steve Workman Date: Thu, 5 Sep 2013 17:07:04 -0700 Subject: [PATCH] Bug 913151 - Always call nsInputStreamPump::OnStateTransfer on the main thread r=jduell --- netwerk/base/src/nsInputStreamPump.cpp | 39 +++++++++++++------------- netwerk/base/src/nsInputStreamPump.h | 14 ++------- 2 files changed, 23 insertions(+), 30 deletions(-) diff --git a/netwerk/base/src/nsInputStreamPump.cpp b/netwerk/base/src/nsInputStreamPump.cpp index bbae50120a3c..65c8a59e5cbc 100644 --- a/netwerk/base/src/nsInputStreamPump.cpp +++ b/netwerk/base/src/nsInputStreamPump.cpp @@ -579,12 +579,10 @@ nsInputStreamPump::OnStateTransfer() } 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; @@ -593,20 +591,17 @@ nsInputStreamPump::OnStateStopForFailure() 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"); @@ -616,6 +611,12 @@ nsInputStreamPump::OnStateStop() // 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) diff --git a/netwerk/base/src/nsInputStreamPump.h b/netwerk/base/src/nsInputStreamPump.h index 526517073dcb..ffc7163bc70c 100644 --- a/netwerk/base/src/nsInputStreamPump.h +++ b/netwerk/base/src/nsInputStreamPump.h @@ -57,18 +57,10 @@ public: 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: