From: Eric Laurent Date: Fri, 6 Sep 2013 01:09:19 +0000 (-0700) Subject: audioflinger: remove async write race conditions X-Git-Tag: android-x86-4.4-r1~142 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=3b4529e0;p=android-x86%2Fframeworks-av.git audioflinger: remove async write race conditions Remove possible race conditions between async callback thread and offload thread when clearing and setting the draining and write blocked flags. Bug: 8174034. Change-Id: I7af10491f39dc0e7d7414862a9d8e763daa2e2b7 --- diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp index fda421157d..e35f47e203 100644 --- a/services/audioflinger/Threads.cpp +++ b/services/audioflinger/Threads.cpp @@ -939,8 +939,8 @@ AudioFlinger::PlaybackThread::PlaybackThread(const sp& audioFlinge mBytesRemaining(0), mCurrentWriteLength(0), mUseAsyncWrite(false), - mWriteBlocked(false), - mDraining(false), + mWriteAckSequence(0), + mDrainSequence(0), mScreenState(AudioFlinger::mScreenState), // index 0 is reserved for normal mixer's submix mFastTrackAvailMask(((1 << FastMixerState::kMaxFastTracks) - 1) & ~1), @@ -1491,29 +1491,31 @@ void AudioFlinger::PlaybackThread::audioConfigChanged_l(int event, int param) { void AudioFlinger::PlaybackThread::writeCallback() { ALOG_ASSERT(mCallbackThread != 0); - mCallbackThread->setWriteBlocked(false); + mCallbackThread->resetWriteBlocked(); } void AudioFlinger::PlaybackThread::drainCallback() { ALOG_ASSERT(mCallbackThread != 0); - mCallbackThread->setDraining(false); + mCallbackThread->resetDraining(); } -void AudioFlinger::PlaybackThread::setWriteBlocked(bool value) +void AudioFlinger::PlaybackThread::resetWriteBlocked(uint32_t sequence) { Mutex::Autolock _l(mLock); - mWriteBlocked = value; - if (!value) { + // reject out of sequence requests + if ((mWriteAckSequence & 1) && (sequence == mWriteAckSequence)) { + mWriteAckSequence &= ~1; mWaitWorkCV.signal(); } } -void AudioFlinger::PlaybackThread::setDraining(bool value) +void AudioFlinger::PlaybackThread::resetDraining(uint32_t sequence) { Mutex::Autolock _l(mLock); - mDraining = value; - if (!value) { + // reject out of sequence requests + if ((mDrainSequence & 1) && (sequence == mDrainSequence)) { + mDrainSequence &= ~1; mWaitWorkCV.signal(); } } @@ -1833,9 +1835,11 @@ ssize_t AudioFlinger::PlaybackThread::threadLoop_write() // Direct output and offload threads size_t offset = (mCurrentWriteLength - mBytesRemaining) / sizeof(int16_t); if (mUseAsyncWrite) { - mWriteBlocked = true; + ALOGW_IF(mWriteAckSequence & 1, "threadLoop_write(): out of sequence write request"); + mWriteAckSequence += 2; + mWriteAckSequence |= 1; ALOG_ASSERT(mCallbackThread != 0); - mCallbackThread->setWriteBlocked(true); + mCallbackThread->setWriteBlocked(mWriteAckSequence); } // FIXME We should have an implementation of timestamps for direct output threads. // They are used e.g for multichannel PCM playback over HDMI. @@ -1844,9 +1848,9 @@ ssize_t AudioFlinger::PlaybackThread::threadLoop_write() if (mUseAsyncWrite && ((bytesWritten < 0) || (bytesWritten == (ssize_t)mBytesRemaining))) { // do not wait for async callback in case of error of full write - mWriteBlocked = false; + mWriteAckSequence &= ~1; ALOG_ASSERT(mCallbackThread != 0); - mCallbackThread->setWriteBlocked(false); + mCallbackThread->setWriteBlocked(mWriteAckSequence); } } @@ -1861,9 +1865,10 @@ void AudioFlinger::PlaybackThread::threadLoop_drain() if (mOutput->stream->drain) { ALOGV("draining %s", (mMixerStatus == MIXER_DRAIN_TRACK) ? "early" : "full"); if (mUseAsyncWrite) { - mDraining = true; + ALOGW_IF(mDrainSequence & 1, "threadLoop_drain(): out of sequence drain request"); + mDrainSequence |= 1; ALOG_ASSERT(mCallbackThread != 0); - mCallbackThread->setDraining(true); + mCallbackThread->setDraining(mDrainSequence); } mOutput->stream->drain(mOutput->stream, (mMixerStatus == MIXER_DRAIN_TRACK) ? AUDIO_DRAIN_EARLY_NOTIFY @@ -2613,11 +2618,12 @@ void AudioFlinger::PlaybackThread::threadLoop_standby() ALOGV("Audio hardware entering standby, mixer %p, suspend count %d", this, mSuspended); mOutput->stream->common.standby(&mOutput->stream->common); if (mUseAsyncWrite != 0) { - mWriteBlocked = false; - mDraining = false; + // discard any pending drain or write ack by incrementing sequence + mWriteAckSequence = (mWriteAckSequence + 2) & ~1; + mDrainSequence = (mDrainSequence + 2) & ~1; ALOG_ASSERT(mCallbackThread != 0); - mCallbackThread->setWriteBlocked(false); - mCallbackThread->setDraining(false); + mCallbackThread->setWriteBlocked(mWriteAckSequence); + mCallbackThread->setDraining(mDrainSequence); } } @@ -3704,8 +3710,8 @@ AudioFlinger::AsyncCallbackThread::AsyncCallbackThread( const sp& offloadThread) : Thread(false /*canCallJava*/), mOffloadThread(offloadThread), - mWriteBlocked(false), - mDraining(false) + mWriteAckSequence(0), + mDrainSequence(0) { } @@ -3721,8 +3727,8 @@ void AudioFlinger::AsyncCallbackThread::onFirstRef() bool AudioFlinger::AsyncCallbackThread::threadLoop() { while (!exitPending()) { - bool writeBlocked; - bool draining; + uint32_t writeAckSequence; + uint32_t drainSequence; { Mutex::Autolock _l(mLock); @@ -3730,18 +3736,21 @@ bool AudioFlinger::AsyncCallbackThread::threadLoop() if (exitPending()) { break; } - writeBlocked = mWriteBlocked; - draining = mDraining; - ALOGV("AsyncCallbackThread mWriteBlocked %d mDraining %d", mWriteBlocked, mDraining); + ALOGV("AsyncCallbackThread mWriteAckSequence %d mDrainSequence %d", + mWriteAckSequence, mDrainSequence); + writeAckSequence = mWriteAckSequence; + mWriteAckSequence &= ~1; + drainSequence = mDrainSequence; + mDrainSequence &= ~1; } { sp offloadThread = mOffloadThread.promote(); if (offloadThread != 0) { - if (writeBlocked == false) { - offloadThread->setWriteBlocked(false); + if (writeAckSequence & 1) { + offloadThread->resetWriteBlocked(writeAckSequence >> 1); } - if (draining == false) { - offloadThread->setDraining(false); + if (drainSequence & 1) { + offloadThread->resetDraining(drainSequence >> 1); } } } @@ -3757,20 +3766,36 @@ void AudioFlinger::AsyncCallbackThread::exit() mWaitWorkCV.broadcast(); } -void AudioFlinger::AsyncCallbackThread::setWriteBlocked(bool value) +void AudioFlinger::AsyncCallbackThread::setWriteBlocked(uint32_t sequence) { Mutex::Autolock _l(mLock); - mWriteBlocked = value; - if (!value) { + // bit 0 is cleared + mWriteAckSequence = sequence << 1; +} + +void AudioFlinger::AsyncCallbackThread::resetWriteBlocked() +{ + Mutex::Autolock _l(mLock); + // ignore unexpected callbacks + if (mWriteAckSequence & 2) { + mWriteAckSequence |= 1; mWaitWorkCV.signal(); } } -void AudioFlinger::AsyncCallbackThread::setDraining(bool value) +void AudioFlinger::AsyncCallbackThread::setDraining(uint32_t sequence) +{ + Mutex::Autolock _l(mLock); + // bit 0 is cleared + mDrainSequence = sequence << 1; +} + +void AudioFlinger::AsyncCallbackThread::resetDraining() { Mutex::Autolock _l(mLock); - mDraining = value; - if (!value) { + // ignore unexpected callbacks + if (mDrainSequence & 2) { + mDrainSequence |= 1; mWaitWorkCV.signal(); } } @@ -3858,7 +3883,7 @@ AudioFlinger::PlaybackThread::mixer_state AudioFlinger::OffloadThread::prepareTr } tracksToRemove->add(track); } else if (track->framesReady() && track->isReady() && - !track->isPaused() && !track->isTerminated()) { + !track->isPaused() && !track->isTerminated() && !track->isStopping_2()) { ALOGVV("OffloadThread: track %d s=%08x [OK]", track->name(), cblk->mServer); if (track->mFillingUpStatus == Track::FS_FILLED) { track->mFillingUpStatus = Track::FS_ACTIVE; @@ -3901,6 +3926,7 @@ AudioFlinger::PlaybackThread::mixer_state AudioFlinger::OffloadThread::prepareTr standbyTime = systemTime() + standbyDelay; if (last) { mixerStatus = MIXER_DRAIN_TRACK; + mDrainSequence += 2; if (mHwPaused) { // It is possible to move from PAUSED to STOPPING_1 without // a resume so we must ensure hardware is running @@ -3911,7 +3937,7 @@ AudioFlinger::PlaybackThread::mixer_state AudioFlinger::OffloadThread::prepareTr } } else if (track->isStopping_2()) { // Drain has completed, signal presentation complete - if (!mDraining || !last) { + if (!(mDrainSequence & 1) || !last) { track->mState = TrackBase::STOPPED; size_t audioHALFrames = (mOutput->stream->get_latency(mOutput->stream)*mSampleRate) / 1000; @@ -3956,8 +3982,9 @@ void AudioFlinger::OffloadThread::flushOutput_l() // must be called with thread mutex locked bool AudioFlinger::OffloadThread::waitingAsyncCallback_l() { - ALOGV("waitingAsyncCallback_l mWriteBlocked %d mDraining %d", mWriteBlocked, mDraining); - if (mUseAsyncWrite && (mWriteBlocked || mDraining)) { + ALOGVV("waitingAsyncCallback_l mWriteAckSequence %d mDrainSequence %d", + mWriteAckSequence, mDrainSequence); + if (mUseAsyncWrite && ((mWriteAckSequence & 1) || (mDrainSequence & 1))) { return true; } return false; @@ -3993,11 +4020,12 @@ void AudioFlinger::OffloadThread::flushHw_l() mPausedWriteLength = 0; mPausedBytesRemaining = 0; if (mUseAsyncWrite) { - mWriteBlocked = false; - mDraining = false; + // discard any pending drain or write ack by incrementing sequence + mWriteAckSequence = (mWriteAckSequence + 2) & ~1; + mDrainSequence = (mDrainSequence + 2) & ~1; ALOG_ASSERT(mCallbackThread != 0); - mCallbackThread->setWriteBlocked(false); - mCallbackThread->setDraining(false); + mCallbackThread->setWriteBlocked(mWriteAckSequence); + mCallbackThread->setDraining(mDrainSequence); } } diff --git a/services/audioflinger/Threads.h b/services/audioflinger/Threads.h index 1333de294a..3fe470ca8a 100644 --- a/services/audioflinger/Threads.h +++ b/services/audioflinger/Threads.h @@ -377,9 +377,9 @@ protected: void removeTracks_l(const Vector< sp >& tracksToRemove); void writeCallback(); - void setWriteBlocked(bool value); + void resetWriteBlocked(uint32_t sequence); void drainCallback(); - void setDraining(bool value); + void resetDraining(uint32_t sequence); static int asyncCallback(stream_callback_event_t event, void *param, void *cookie); @@ -577,8 +577,19 @@ private: size_t mBytesRemaining; size_t mCurrentWriteLength; bool mUseAsyncWrite; - bool mWriteBlocked; - bool mDraining; + // mWriteAckSequence contains current write sequence on bits 31-1. The write sequence is + // incremented each time a write(), a flush() or a standby() occurs. + // Bit 0 is set when a write blocks and indicates a callback is expected. + // Bit 0 is reset by the async callback thread calling resetWriteBlocked(). Out of sequence + // callbacks are ignored. + uint32_t mWriteAckSequence; + // mDrainSequence contains current drain sequence on bits 31-1. The drain sequence is + // incremented each time a drain is requested or a flush() or standby() occurs. + // Bit 0 is set when the drain() command is called at the HAL and indicates a callback is + // expected. + // Bit 0 is reset by the async callback thread calling resetDraining(). Out of sequence + // callbacks are ignored. + uint32_t mDrainSequence; bool mSignalPending; sp mCallbackThread; @@ -755,13 +766,21 @@ public: virtual void onFirstRef(); void exit(); - void setWriteBlocked(bool value); - void setDraining(bool value); + void setWriteBlocked(uint32_t sequence); + void resetWriteBlocked(); + void setDraining(uint32_t sequence); + void resetDraining(); private: wp mOffloadThread; - bool mWriteBlocked; - bool mDraining; + // mWriteAckSequence corresponds to the last write sequence passed by the offload thread via + // setWriteBlocked(). The sequence is shifted one bit to the left and the lsb is used + // to indicate that the callback has been received via resetWriteBlocked() + uint32_t mWriteAckSequence; + // mDrainSequence corresponds to the last drain sequence passed by the offload thread via + // setDraining(). The sequence is shifted one bit to the left and the lsb is used + // to indicate that the callback has been received via resetDraining() + uint32_t mDrainSequence; Condition mWaitWorkCV; Mutex mLock; };