Bug 1072775: Additional assertions for MediaStreamGraph/GraphDriver r=roc
--- a/content/media/GraphDriver.cpp
+++ b/content/media/GraphDriver.cpp
@@ -255,16 +255,17 @@ void
ThreadedDriver::Stop()
{
NS_ASSERTION(NS_IsMainThread(), "Must be called on main thread");
// mGraph's thread is not running so it's OK to do whatever here
STREAM_LOG(PR_LOG_DEBUG, ("Stopping threads for MediaStreamGraph %p", this));
if (mThread) {
mThread->Shutdown();
+ mThread = nullptr;
}
}
SystemClockDriver::SystemClockDriver(MediaStreamGraphImpl* aGraphImpl)
: ThreadedDriver(aGraphImpl),
mInitialTimeStamp(TimeStamp::Now()),
mLastTimeStamp(TimeStamp::Now())
{}
@@ -395,29 +396,35 @@ class MediaStreamGraphShutdownThreadRunn
public:
explicit MediaStreamGraphShutdownThreadRunnable2(nsIThread* aThread)
: mThread(aThread)
{
}
NS_IMETHOD Run()
{
MOZ_ASSERT(NS_IsMainThread());
+ MOZ_ASSERT(mThread);
+
mThread->Shutdown();
+ mThread = nullptr;
return NS_OK;
}
private:
- nsRefPtr<nsIThread> mThread;
+ nsCOMPtr<nsIThread> mThread;
};
OfflineClockDriver::~OfflineClockDriver()
{
// transfer the ownership of mThread to the event
- nsCOMPtr<nsIRunnable> event = new MediaStreamGraphShutdownThreadRunnable2(mThread);
- mThread = nullptr;
- NS_DispatchToMainThread(event);
+ // XXX should use .forget()/etc
+ if (mThread) {
+ nsCOMPtr<nsIRunnable> event = new MediaStreamGraphShutdownThreadRunnable2(mThread);
+ mThread = nullptr;
+ NS_DispatchToMainThread(event);
+ }
}
void
OfflineClockDriver::GetIntervalForIteration(GraphTime& aFrom, GraphTime& aTo)
{
aFrom = mIterationStart = IterationEnd();
aTo = mIterationEnd = IterationEnd() + mGraphImpl->MillisecondsToMediaTime(mSlice);
@@ -736,29 +743,26 @@ AudioCallbackDriver::StateCallback_s(cub
/* static */ void
AudioCallbackDriver::DeviceChangedCallback_s(void* aUser)
{
AudioCallbackDriver* driver = reinterpret_cast<AudioCallbackDriver*>(aUser);
driver->DeviceChangedCallback();
}
bool AudioCallbackDriver::InCallback() {
- MonitorAutoLock mon(mGraphImpl->GetMonitor());
return mInCallback;
}
AudioCallbackDriver::AutoInCallback::AutoInCallback(AudioCallbackDriver* aDriver)
: mDriver(aDriver)
{
- MonitorAutoLock mon(mDriver->mGraphImpl->GetMonitor());
mDriver->mInCallback = true;
}
AudioCallbackDriver::AutoInCallback::~AutoInCallback() {
- MonitorAutoLock mon(mDriver->mGraphImpl->GetMonitor());
mDriver->mInCallback = false;
}
long
AudioCallbackDriver::DataCallback(AudioDataValue* aBuffer, long aFrames)
{
bool stillProcessing;
--- a/content/media/GraphDriver.h
+++ b/content/media/GraphDriver.h
@@ -6,16 +6,17 @@
#ifndef GRAPHDRIVER_H_
#define GRAPHDRIVER_H_
#include "nsAutoPtr.h"
#include "nsAutoRef.h"
#include "AudioBufferUtils.h"
#include "AudioMixer.h"
#include "AudioSegment.h"
+#include "mozilla/Atomics.h"
struct cubeb_stream;
template <>
class nsAutoRefTraits<cubeb_stream> : public nsPointerRefTraits<cubeb_stream>
{
public:
static void Release(cubeb_stream* aStream) { cubeb_stream_destroy(aStream); }
@@ -193,16 +194,18 @@ public:
* Same thing, but not locked.
*/
void EnsureNextIterationLocked();
MediaStreamGraphImpl* GraphImpl() {
return mGraphImpl;
}
+ virtual bool OnThread() = 0;
+
protected:
// Time of the start of this graph iteration.
GraphTime mIterationStart;
// Time of the end of this graph iteration.
GraphTime mIterationEnd;
// Time, in the future, for which blocking has been computed.
GraphTime mStateComputedTime;
GraphTime mNextStateComputedTime;
@@ -257,16 +260,19 @@ public:
* Runs main control loop on the graph thread. Normally a single invocation
* of this runs for the entire lifetime of the graph thread.
*/
void RunThread();
friend class MediaStreamGraphInitThreadRunnable;
uint32_t IterationDuration() {
return MEDIA_GRAPH_TARGET_PERIOD_MS;
}
+
+ virtual bool OnThread() MOZ_OVERRIDE { return !mThread || NS_GetCurrentThread() == mThread; }
+
protected:
nsCOMPtr<nsIThread> mThread;
};
/**
* A SystemClockDriver drives a MediaStreamGraph using a system clock, and waits
* using a monitor, between each iteration.
*/
@@ -379,16 +385,18 @@ public:
return this;
}
/**
* Whether the audio callback is processing. This is for asserting only.
*/
bool InCallback();
+ virtual bool OnThread() MOZ_OVERRIDE { return !mStarted || InCallback(); }
+
/* Whether the underlying cubeb stream has been started. See comment for
* mStarted for details. */
bool IsStarted();
/* Tell the driver whether this process is using a microphone or not. This is
* thread safe. */
void SetMicrophoneActive(bool aActive);
private:
@@ -444,18 +452,17 @@ private:
~AutoInCallback();
AudioCallbackDriver* mDriver;
};
/* Thread for off-main-thread initialization and
* shutdown of the audio stream. */
nsCOMPtr<nsIThread> mInitShutdownThread;
dom::AudioChannel mAudioChannel;
- /* This can only be accessed with the graph's monitor held. */
- bool mInCallback;
+ Atomic<bool> mInCallback;
/* A thread has been created to be able to pause and restart the audio thread,
* but has not done so yet. This indicates that the callback should return
* early */
bool mPauseRequested;
/**
* True if microphone is being used by this process. This is synchronized by
* the graph's monitor. */
bool mMicrophoneActive;
--- a/content/media/MediaStreamGraph.cpp
+++ b/content/media/MediaStreamGraph.cpp
@@ -1481,21 +1481,28 @@ public:
{}
NS_IMETHOD Run()
{
NS_ASSERTION(mGraph->mDetectedNotRunning,
"We should know the graph thread control loop isn't running!");
LIFECYCLE_LOG("Shutting down graph %p", mGraph.get());
- if (mGraph->CurrentDriver()->AsAudioCallbackDriver()) {
- MOZ_ASSERT(!mGraph->CurrentDriver()->AsAudioCallbackDriver()->InCallback());
+ // We've asserted the graph isn't running. Use mDriver instead of CurrentDriver
+ // to avoid thread-safety checks
+#if 0 // AudioCallbackDrivers are released asynchronously anyways
+ // XXX a better test would be have setting mDetectedNotRunning make sure
+ // any current callback has finished and block future ones -- or just
+ // handle it all in Shutdown()!
+ if (mGraph->mDriver->AsAudioCallbackDriver()) {
+ MOZ_ASSERT(!mGraph->mDriver->AsAudioCallbackDriver()->InCallback());
}
+#endif
- mGraph->CurrentDriver()->Shutdown();
+ mGraph->mDriver->Shutdown();
// mGraph's thread is not running so it's OK to do whatever here
if (mGraph->IsEmpty()) {
// mGraph is no longer needed, so delete it.
mGraph->Destroy();
} else {
// The graph is not empty. We must be in a forced shutdown, or a
// non-realtime graph that has finished processing. Some later
@@ -1715,16 +1722,17 @@ MediaStreamGraphImpl::RunInStableState(b
}
#ifdef DEBUG
mCanRunMessagesSynchronously = mDetectedNotRunning &&
mLifecycleState >= LIFECYCLE_WAITING_FOR_THREAD_SHUTDOWN;
#endif
}
+
static NS_DEFINE_CID(kAppShellCID, NS_APPSHELL_CID);
void
MediaStreamGraphImpl::EnsureRunInStableState()
{
NS_ASSERTION(NS_IsMainThread(), "main thread only");
if (mPostedRunInStableState)
--- a/content/media/MediaStreamGraphImpl.h
+++ b/content/media/MediaStreamGraphImpl.h
@@ -145,16 +145,22 @@ public:
void ShutdownThreads();
/**
* Called before the thread runs.
*/
void Init();
// The following methods run on the graph thread (or possibly the main thread if
// mLifecycleState > LIFECYCLE_RUNNING)
+ void AssertOnGraphThreadOrNotRunning() {
+ // 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
+ MOZ_ASSERT(mDriver->OnThread() ||
+ (mLifecycleState > LIFECYCLE_RUNNING && NS_IsMainThread()));
+ }
/*
* This does the actual iteration: Message processing, MediaStream ordering,
* blocking computation and processing.
*/
void DoIteration();
bool OneIteration(GraphTime aFrom, GraphTime aTo,
GraphTime aStateFrom, GraphTime aStateEnd);
@@ -413,26 +419,33 @@ public:
*/
void PausedIndefinitly();
void ResumedFromPaused();
/**
* Not safe to call off the MediaStreamGraph thread unless monitor is held!
*/
GraphDriver* CurrentDriver() {
+#ifdef DEBUG
+ // #ifdef since we're not wrapping it all in MOZ_ASSERT()
+ if (mDriver->OnThread()) {
+ mMonitor.AssertCurrentThreadOwns();
+ }
+#endif
return mDriver;
}
/**
* Effectively set the new driver, while we are switching.
* 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.
*/
void SetCurrentDriver(GraphDriver* aDriver) {
+ MOZ_ASSERT(mDriver->OnThread());
mDriver = aDriver;
}
Monitor& GetMonitor() {
return mMonitor;
}
/**