Bug 1072780: patch 1 - clean up CurrentDriver() use off-MSG-thread; fix InCallback() r=roc
--- a/content/media/GraphDriver.cpp
+++ b/content/media/GraphDriver.cpp
@@ -762,17 +762,22 @@ AudioCallbackDriver::DataCallback(AudioD
{
bool stillProcessing;
if (mPauseRequested) {
PodZero(aBuffer, aFrames * mGraphImpl->AudioChannelCount());
return aFrames;
}
- DebugOnly<AutoInCallback> aic(AutoInCallback(this));
+#ifdef DEBUG
+ // DebugOnly<> doesn't work here... it forces an initialization that will cause
+ // mInCallback to be set back to false before we exit the statement. Do it by
+ // hand instead.
+ AutoInCallback aic(this);
+#endif
if (mStateComputedTime == 0) {
MonitorAutoLock mon(mGraphImpl->GetMonitor());
// Because this function is called during cubeb_stream_init (to prefill the
// audio buffers), it can be that we don't have a message here (because this
// driver is the first one for this graph), and the graph would exit. Simply
// return here until we have messages.
if (!mGraphImpl->MessagesQueued()) {
--- a/content/media/MediaStreamGraph.cpp
+++ b/content/media/MediaStreamGraph.cpp
@@ -1646,42 +1646,44 @@ MediaStreamGraphImpl::RunInStableState(b
// processing.
if (mLifecycleState == LIFECYCLE_WAITING_FOR_MAIN_THREAD_CLEANUP &&
mRealtime && !mForceShutDown) {
mLifecycleState = LIFECYCLE_RUNNING;
// Revive the MediaStreamGraph since we have more messages going to it.
// Note that we need to put messages into its queue before reviving it,
// or it might exit immediately.
{
- MonitorAutoUnlock unlock(mMonitor);
LIFECYCLE_LOG("Reviving a graph (%p) ! %s\n",
this, CurrentDriver()->AsAudioCallbackDriver() ? "AudioDriver" :
"SystemDriver");
- CurrentDriver()->Revive();
+ nsRefPtr<GraphDriver> driver = CurrentDriver();
+ MonitorAutoUnlock unlock(mMonitor);
+ driver->Revive();
}
}
}
// Don't start the thread for a non-realtime graph until it has been
// explicitly started by StartNonRealtimeProcessing.
if (mLifecycleState == LIFECYCLE_THREAD_NOT_STARTED &&
(mRealtime || mNonRealtimeProcessing)) {
mLifecycleState = LIFECYCLE_RUNNING;
// Start the thread now. We couldn't start it earlier because
// the graph might exit immediately on finding it has no streams. The
// first message for a new graph must create a stream.
{
// We should exit the monitor for now, because starting a stream might
// take locks, and we don't want to deadlock.
- MonitorAutoUnlock unlock(mMonitor);
LIFECYCLE_LOG("Starting a graph (%p) ! %s\n",
this,
CurrentDriver()->AsAudioCallbackDriver() ? "AudioDriver" :
"SystemDriver");
- CurrentDriver()->Start();
+ nsRefPtr<GraphDriver> driver = CurrentDriver();
+ MonitorAutoUnlock unlock(mMonitor);
+ driver->Start();
}
}
if ((mForceShutDown || !mRealtime) &&
mLifecycleState == LIFECYCLE_WAITING_FOR_MAIN_THREAD_CLEANUP) {
// Defer calls to RunDuringShutdown() to happen while mMonitor is not held.
for (uint32_t i = 0; i < mBackMessageQueue.Length(); ++i) {
MessageBlock& mb = mBackMessageQueue[i];
@@ -2253,17 +2255,17 @@ SourceMediaStream::DestroyImpl()
}
void
SourceMediaStream::SetPullEnabled(bool aEnabled)
{
MutexAutoLock lock(mMutex);
mPullEnabled = aEnabled;
if (mPullEnabled && GraphImpl()) {
- GraphImpl()->CurrentDriver()->EnsureNextIteration();
+ GraphImpl()->EnsureNextIteration();
}
}
void
SourceMediaStream::AddTrack(TrackID aID, TrackRate aRate, TrackTicks aStart,
MediaSegment* aSegment)
{
MutexAutoLock lock(mMutex);
@@ -2273,17 +2275,17 @@ SourceMediaStream::AddTrack(TrackID aID,
// We resample all audio input tracks to the sample rate of the audio mixer.
data->mOutputRate = aSegment->GetType() == MediaSegment::AUDIO ?
GraphImpl()->AudioSampleRate() : aRate;
data->mStart = aStart;
data->mCommands = TRACK_CREATE;
data->mData = aSegment;
data->mHaveEnough = false;
if (auto graph = GraphImpl()) {
- graph->CurrentDriver()->EnsureNextIteration();
+ graph->EnsureNextIteration();
}
}
void
SourceMediaStream::ResampleAudioToGraphSampleRate(TrackData* aTrackData, MediaSegment* aSegment)
{
if (aSegment->GetType() != MediaSegment::AUDIO ||
aTrackData->mInputRate == GraphImpl()->AudioSampleRate()) {
@@ -2335,17 +2337,17 @@ SourceMediaStream::AppendToTrack(TrackID
ApplyTrackDisabling(aID, aSegment, aRawSegment);
ResampleAudioToGraphSampleRate(track, aSegment);
// Must notify first, since AppendFrom() will empty out aSegment
NotifyDirectConsumers(track, aRawSegment ? aRawSegment : aSegment);
track->mData->AppendFrom(aSegment); // note: aSegment is now dead
appended = true;
- GraphImpl()->CurrentDriver()->EnsureNextIteration();
+ GraphImpl()->EnsureNextIteration();
} else {
aSegment->Clear();
}
}
return appended;
}
void
@@ -2459,38 +2461,38 @@ SourceMediaStream::EndTrack(TrackID aID)
// ::EndAllTrackAndFinished() can end these before the sources call this
if (!mFinished) {
TrackData *track = FindDataForTrack(aID);
if (track) {
track->mCommands |= TRACK_END;
}
}
if (auto graph = GraphImpl()) {
- graph->CurrentDriver()->EnsureNextIteration();
+ graph->EnsureNextIteration();
}
}
void
SourceMediaStream::AdvanceKnownTracksTime(StreamTime aKnownTime)
{
MutexAutoLock lock(mMutex);
MOZ_ASSERT(aKnownTime >= mUpdateKnownTracksTime);
mUpdateKnownTracksTime = aKnownTime;
if (auto graph = GraphImpl()) {
- graph->CurrentDriver()->EnsureNextIteration();
+ graph->EnsureNextIteration();
}
}
void
SourceMediaStream::FinishWithLockHeld()
{
mMutex.AssertCurrentThreadOwns();
mUpdateFinished = true;
if (auto graph = GraphImpl()) {
- graph->CurrentDriver()->EnsureNextIteration();
+ graph->EnsureNextIteration();
}
}
void
SourceMediaStream::EndAllTrackAndFinish()
{
MutexAutoLock lock(mMutex);
for (uint32_t i = 0; i < mUpdateTracks.Length(); ++i) {
--- a/content/media/MediaStreamGraphImpl.h
+++ b/content/media/MediaStreamGraphImpl.h
@@ -409,16 +409,19 @@ public:
/**
* Signal to the graph that the thread has paused indefinitly,
* or resumed.
*/
void PausedIndefinitly();
void ResumedFromPaused();
+ /**
+ * Not safe to call off the MediaStreamGraph thread unless monitor is held!
+ */
GraphDriver* CurrentDriver() {
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
@@ -427,16 +430,26 @@ public:
void SetCurrentDriver(GraphDriver* aDriver) {
mDriver = aDriver;
}
Monitor& GetMonitor() {
return mMonitor;
}
+ /**
+ * Must implement here to avoid dangerous data races around CurrentDriver() -
+ * we don't want stuff off MSG thread using "graph->CurrentDriver()->EnsureNextIteration()"
+ * because CurrentDriver may change (and it's a TSAN data race)
+ */
+ void EnsureNextIteration() {
+ MonitorAutoLock mon(mMonitor);
+ CurrentDriver()->EnsureNextIterationLocked();
+ }
+
// Data members
//
/**
* Graphs own owning references to their driver, until shutdown. When a driver
* switch occur, previous driver is either deleted, or it's ownership is
* passed to a event that will take care of the asynchronous cleanup, as
* audio stream can take some time to shut down.
*/