bug 1406831 don't tolerate just owning the monitor if AssertOnGraphThreadOrNotRunning() is not called on the correct thread r?pehrsons
Owning the monitor is not sufficient for consistent state if state can be
accessed without the monitor.
The requirements for SetCurrentDriver() are tighted to require both the
monitor and correct thread, as CurrentDriver() can be called either with the
monitor or on the graph thread.
MozReview-Commit-ID: 90q7Pfa8jxn
--- a/dom/media/MediaStreamGraph.cpp
+++ b/dom/media/MediaStreamGraph.cpp
@@ -1110,31 +1110,27 @@ void
MediaStreamGraph::NotifyOutputData(AudioDataValue* aBuffer, size_t aFrames,
TrackRate aRate, uint32_t aChannels)
{
for (auto& listener : mAudioInputs) {
listener->NotifyOutputData(this, aBuffer, aFrames, aRate, aChannels);
}
}
-void
-MediaStreamGraph::AssertOnGraphThreadOrNotRunning() const
+bool
+MediaStreamGraph::OnGraphThreadOrNotRunning() const
{
// either we're on the right thread (and calling CurrentDriver() is safe),
- // or we're going to assert anyways, so don't cross-check CurrentDriver
-#ifdef DEBUG
+ // or we're going to fail the assert anyway, so don't cross-check
+ // via CurrentDriver().
MediaStreamGraphImpl const * graph =
static_cast<MediaStreamGraphImpl const *>(this);
// if all the safety checks fail, assert we own the monitor
- if (!(graph->mDetectedNotRunning ?
- NS_IsMainThread() : graph->mDriver->OnThread()))
- {
- graph->mMonitor.AssertCurrentThreadOwns();
- }
-#endif
+ return graph->mDetectedNotRunning ?
+ NS_IsMainThread() : graph->mDriver->OnThread();
}
bool
MediaStreamGraphImpl::ShouldUpdateMainThread()
{
if (mRealtime) {
return true;
}
--- a/dom/media/MediaStreamGraph.h
+++ b/dom/media/MediaStreamGraph.h
@@ -1362,29 +1362,36 @@ public:
/**
* Data going to the speakers from the GraphDriver's DataCallback
* to notify any listeners (for echo cancellation).
*/
void NotifyOutputData(AudioDataValue* aBuffer, size_t aFrames,
TrackRate aRate, uint32_t aChannels);
- void AssertOnGraphThreadOrNotRunning() const;
+ void AssertOnGraphThreadOrNotRunning() const
+ {
+ MOZ_ASSERT(OnGraphThreadOrNotRunning());
+ }
protected:
explicit MediaStreamGraph(TrackRate aSampleRate)
: mSampleRate(aSampleRate)
{
MOZ_COUNT_CTOR(MediaStreamGraph);
}
virtual ~MediaStreamGraph()
{
MOZ_COUNT_DTOR(MediaStreamGraph);
}
+ // Intended only for assertions, either on graph thread, not running, or
+ // with monitor held.
+ bool OnGraphThreadOrNotRunning() const;
+
// Media graph thread only
nsTArray<nsCOMPtr<nsIRunnable> > mPendingUpdateRunnables;
/**
* Sample rate at which this graph runs. For real time graphs, this is
* the rate of the audio mixer. For offline graphs, this is the rate specified
* at construction.
*/
--- a/dom/media/MediaStreamGraphImpl.h
+++ b/dom/media/MediaStreamGraphImpl.h
@@ -451,17 +451,21 @@ public:
void PausedIndefinitly();
void ResumedFromPaused();
/**
* Not safe to call off the MediaStreamGraph thread unless monitor is held!
*/
GraphDriver* CurrentDriver() const
{
- AssertOnGraphThreadOrNotRunning();
+#ifdef DEBUG
+ if (!OnGraphThreadOrNotRunning()) {
+ mMonitor.AssertCurrentThreadOwns();
+ }
+#endif
return mDriver;
}
bool RemoveMixerCallback(MixerCallbackReceiver* aReceiver)
{
return mMixer.RemoveCallback(aReceiver);
}
@@ -470,17 +474,20 @@ public:
* It is only safe to call this at the very end of an iteration, when there
* has been a SwitchAtNextIteration call during the iteration. The driver
* should return and pass the control to the new driver shortly after.
* We can also switch from Revive() (on MainThread), in which case the
* monitor is held
*/
void SetCurrentDriver(GraphDriver* aDriver)
{
+#ifdef DEBUG
+ mMonitor.AssertCurrentThreadOwns();
AssertOnGraphThreadOrNotRunning();
+#endif
mDriver = aDriver;
}
Monitor& GetMonitor()
{
return mMonitor;
}