Use atomic ops for thread suspend count
There was a theoretical but unlikely race if two binder threads
executed suspend() or restore() concurrently. Also added comments.
Change-Id: I0908acc810b83bdd66455b27ca3429de1662a2cd
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index fc4969e..e2b29f7 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -2513,7 +2513,7 @@
// put audio hardware into standby after short delay
if (CC_UNLIKELY((!mActiveTracks.size() && systemTime() > standbyTime) ||
- mSuspended > 0)) {
+ isSuspended())) {
if (!mStandby) {
threadLoop_standby();
@@ -2567,7 +2567,7 @@
threadLoop_sleepTime();
}
- if (mSuspended > 0) {
+ if (isSuspended()) {
sleepTime = suspendSleepTimeUs();
}
@@ -2752,7 +2752,7 @@
// shared by MIXER and DIRECT, overridden by DUPLICATING
void AudioFlinger::PlaybackThread::threadLoop_standby()
{
- ALOGV("Audio hardware entering standby, mixer %p, suspend count %u", this, mSuspended);
+ ALOGV("Audio hardware entering standby, mixer %p, suspend count %d", this, mSuspended);
mOutput->stream->common.standby(&mOutput->stream->common);
}
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index 648a8d2..a6fd0a0 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -1023,9 +1023,19 @@
AudioStreamOut* clearOutput();
virtual audio_stream_t* stream() const;
- void suspend() { mSuspended++; }
- void restore() { if (mSuspended > 0) mSuspended--; }
- bool isSuspended() const { return (mSuspended > 0); }
+ // a very large number of suspend() will eventually wraparound, but unlikely
+ void suspend() { (void) android_atomic_inc(&mSuspended); }
+ void restore()
+ {
+ // if restore() is done without suspend(), get back into
+ // range so that the next suspend() will operate correctly
+ if (android_atomic_dec(&mSuspended) <= 0) {
+ android_atomic_release_store(0, &mSuspended);
+ }
+ }
+ bool isSuspended() const
+ { return android_atomic_acquire_load(&mSuspended) > 0; }
+
virtual String8 getParameters(const String8& keys);
virtual void audioConfigChanged_l(int event, int param = 0);
status_t getRenderPosition(uint32_t *halFrames, uint32_t *dspFrames);
@@ -1050,7 +1060,14 @@
protected:
int16_t* mMixBuffer;
- uint32_t mSuspended; // suspend count, > 0 means suspended
+
+ // suspend count, > 0 means suspended. While suspended, the thread continues to pull from
+ // tracks and mix, but doesn't write to HAL. A2DP and SCO HAL implementations can't handle
+ // concurrent use of both of them, so Audio Policy Service suspends one of the threads to
+ // workaround that restriction.
+ // 'volatile' means accessed via atomic operations and no lock.
+ volatile int32_t mSuspended;
+
int mBytesWritten;
private:
// mMasterMute is in both PlaybackThread and in AudioFlinger. When a