Bug 1426751 - Prevent FlushQueue to be invoked on multiple threads simultaneously. r?mayhemer
This issue is triggered by off-main-thread ODA listener that return error cause from OnDataAvailable callback.
A CancelEvent will be prepend to event queue and trigger race condition between CompleteResume and EndForceEnqueueing.
The `mFlushing` is checked and set in separate critical sections, therefore two threads that executing MayFlushQueue
might both pass the `mFlushing` check and trying to call FlushQueue simultaneously.
The solution is to check and set `mFlushing` in single critical section, so we can guarantee that only one FlushQueue
can be executed at anytime.
In addition, resumption is postponed until no AutoEventEnqueuer is activated. Therefore, CompleteResume will only be
triggered while all the suspension requests and auto enqueue requests are finished.
MozReview-Commit-ID: HpxzgUqYm8C
--- a/netwerk/ipc/ChannelEventQueue.cpp
+++ b/netwerk/ipc/ChannelEventQueue.cpp
@@ -35,22 +35,22 @@ void
ChannelEventQueue::FlushQueue()
{
// Events flushed could include destruction of channel (and our own
// destructor) unless we make sure its refcount doesn't drop to 0 while this
// method is running.
nsCOMPtr<nsISupports> kungFuDeathGrip(mOwner);
mozilla::Unused << kungFuDeathGrip; // Not used in this function
- // Prevent flushed events from flushing the queue recursively
+#ifdef DEBUG
{
MutexAutoLock lock(mMutex);
- MOZ_ASSERT(!mFlushing);
- mFlushing = true;
+ MOZ_ASSERT(mFlushing);
}
+#endif // DEBUG
bool needResumeOnOtherThread = false;
while (true) {
UniquePtr<ChannelEvent> event;
{
MutexAutoLock lock(mMutex);
event.reset(TakeEvent());
@@ -132,18 +132,19 @@ ChannelEventQueue::ResumeInternal()
// Resuming w/o suspend: error in debug mode, ignore in build
MOZ_ASSERT(mSuspendCount > 0);
if (mSuspendCount <= 0) {
return;
}
if (!--mSuspendCount) {
- if (mEventQueue.IsEmpty()) {
- // Nothing in queue to flush, simply clear the flag.
+ if (mEventQueue.IsEmpty() || !!mForcedCount) {
+ // Nothing in queue to flush or waiting for AutoEventEnqueuer to
+ // finish the force enqueue period, simply clear the flag.
mSuspended = false;
return;
}
// Hold a strong reference of mOwner to avoid the channel release
// before CompleteResume was executed.
class CompleteResumeRunnable : public CancelableRunnable
{
--- a/netwerk/ipc/ChannelEventQueue.h
+++ b/netwerk/ipc/ChannelEventQueue.h
@@ -313,16 +313,21 @@ ChannelEventQueue::MaybeFlushQueue()
// Don't flush if forced queuing on, we're already being flushed, or
// suspended, or there's nothing to flush
bool flushQueue = false;
{
MutexAutoLock lock(mMutex);
flushQueue = !mForcedCount && !mFlushing && !mSuspended &&
!mEventQueue.IsEmpty();
+
+ // Only one thread is allowed to run FlushQueue at a time.
+ if (flushQueue) {
+ mFlushing = true;
+ }
}
if (flushQueue) {
FlushQueue();
}
}
// Ensures that RunOrEnqueue() will be collecting events during its lifetime