From 6f244480a89bea9118094d5d432e804e526cb75f Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Wed, 22 Jul 2020 22:26:34 +0000 Subject: [PATCH] Bug 1612308 - Add a comment to BodyStream explaining why an expected state can't be asserted in a particular place. r=bzbarsky Differential Revision: https://phabricator.services.mozilla.com/D62409 --- dom/base/BodyStream.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/dom/base/BodyStream.cpp b/dom/base/BodyStream.cpp index 812e55e82782..19faf78ec56b 100644 --- a/dom/base/BodyStream.cpp +++ b/dom/base/BodyStream.cpp @@ -12,6 +12,7 @@ #include "mozilla/dom/WorkerPrivate.h" #include "mozilla/dom/WorkerRunnable.h" #include "mozilla/Maybe.h" +#include "mozilla/Unused.h" #include "nsProxyRelease.h" #include "nsStreamUtils.h" @@ -361,6 +362,7 @@ BodyStream::OnInputStreamReady(nsIAsyncInputStream* aStream) { AssertIsOnOwningThread(); MOZ_DIAGNOSTIC_ASSERT(aStream); + // Acquire |mMutex| in order to safely inspect |mState| and use |mGlobal|. Maybe lock; lock.emplace(mMutex); @@ -369,6 +371,11 @@ BodyStream::OnInputStreamReady(nsIAsyncInputStream* aStream) { return NS_OK; } + // Perform a microtask checkpoint after all actions are completed. Note that + // |mMutex| *must not* be held when the checkpoint occurs -- hence, far down, + // the |lock.reset()|. (|MutexAutoUnlock| as RAII wouldn't work for this task + // because its destructor would reacquire |mMutex| before these objects' + // destructors run.) nsAutoMicroTask mt; AutoEntryScript aes(mGlobal, "fetch body data available"); @@ -405,10 +412,15 @@ BodyStream::OnInputStreamReady(nsIAsyncInputStream* aStream) { mState = eWriting; + // Release the mutex before the call below (which could execute JS), as well + // as before the microtask checkpoint queued up above occurs. lock.reset(); - DebugOnly ok = - JS::ReadableStreamUpdateDataAvailableFromSource(cx, stream, size); + Unused << JS::ReadableStreamUpdateDataAvailableFromSource(cx, stream, size); + + // The previous call can execute JS (even up to running a nested event loop), + // so |mState| can't be asserted to have any particular value, even if the + // previous call succeeds. return NS_OK; }