Fix races related to volume and mute
Fix race conditions when setting master volume, master mute, stream
volume, stream mute for a playback thread, and when reading stream
volume of a playback thread. Lock order is AudioFlinger, then thread.
Rename streamVolumeInternal to streamVolume_l, comment, and use it to
implement streamVolume().
Code size reduction:
- Remove dead code: AudioFlinger::PlaybackThread::masterVolume, masterMute, streamMute.
- Change return type of non-binder methods that always succeed from status_t to void.
- Remove virtual from volume and mute methods that don't need it.
This change saves 228 bytes but decreases performance of binder operations
due to the added locks.
Change-Id: Iac75abc1f54784873a667d1981b2e08f8f31e5c9
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index 93c91fb..4cf570c 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -729,7 +729,7 @@
}
volume = thread->streamVolume(stream);
} else {
- volume = mStreamTypes[stream].volume;
+ volume = streamVolume_l(stream);
}
return volume;
@@ -741,7 +741,8 @@
return true;
}
- return mStreamTypes[stream].mute;
+ AutoMutex lock(mLock);
+ return streamMute_l(stream);
}
status_t AudioFlinger::setParameters(audio_io_handle_t ioHandle, const String8& keyValuePairs)
@@ -1383,11 +1384,13 @@
// There is no AUDIO_STREAM_MIN, and ++ operator does not compile
for (audio_stream_type_t stream = (audio_stream_type_t) 0; stream < AUDIO_STREAM_CNT;
stream = (audio_stream_type_t) (stream + 1)) {
- mStreamTypes[stream].volume = mAudioFlinger->streamVolumeInternal(stream);
- mStreamTypes[stream].mute = mAudioFlinger->streamMute(stream);
+ mStreamTypes[stream].volume = mAudioFlinger->streamVolume_l(stream);
+ mStreamTypes[stream].mute = mAudioFlinger->streamMute_l(stream);
// initialized by stream_type_t default constructor
// mStreamTypes[stream].valid = true;
}
+ // mStreamTypes[AUDIO_STREAM_CNT] exists but isn't explicitly initialized here,
+ // because mAudioFlinger doesn't have one to copy from
}
AudioFlinger::PlaybackThread::~PlaybackThread()
@@ -1582,40 +1585,36 @@
}
}
-status_t AudioFlinger::PlaybackThread::setMasterVolume(float value)
+void AudioFlinger::PlaybackThread::setMasterVolume(float value)
{
+ Mutex::Autolock _l(mLock);
mMasterVolume = value;
- return NO_ERROR;
}
-status_t AudioFlinger::PlaybackThread::setMasterMute(bool muted)
+void AudioFlinger::PlaybackThread::setMasterMute(bool muted)
{
- mMasterMute = muted;
- return NO_ERROR;
+ Mutex::Autolock _l(mLock);
+ setMasterMute_l(muted);
}
-status_t AudioFlinger::PlaybackThread::setStreamVolume(audio_stream_type_t stream, float value)
+void AudioFlinger::PlaybackThread::setStreamVolume(audio_stream_type_t stream, float value)
{
+ Mutex::Autolock _l(mLock);
mStreamTypes[stream].volume = value;
- return NO_ERROR;
}
-status_t AudioFlinger::PlaybackThread::setStreamMute(audio_stream_type_t stream, bool muted)
+void AudioFlinger::PlaybackThread::setStreamMute(audio_stream_type_t stream, bool muted)
{
+ Mutex::Autolock _l(mLock);
mStreamTypes[stream].mute = muted;
- return NO_ERROR;
}
float AudioFlinger::PlaybackThread::streamVolume(audio_stream_type_t stream) const
{
+ Mutex::Autolock _l(mLock);
return mStreamTypes[stream].volume;
}
-bool AudioFlinger::PlaybackThread::streamMute(audio_stream_type_t stream) const
-{
- return mStreamTypes[stream].mute;
-}
-
// addTrack_l() must be called with ThreadBase::mLock held
status_t AudioFlinger::PlaybackThread::addTrack_l(const sp<Track>& track)
{
@@ -1945,7 +1944,7 @@
property_get("ro.audio.silent", value, "0");
if (atoi(value)) {
ALOGD("Silence is golden");
- setMasterMute(true);
+ setMasterMute_l(true);
}
}
@@ -2661,7 +2660,7 @@
property_get("ro.audio.silent", value, "0");
if (atoi(value)) {
ALOGD("Silence is golden");
- setMasterMute(true);
+ setMasterMute_l(true);
}
}
@@ -3057,7 +3056,7 @@
property_get("ro.audio.silent", value, "0");
if (atoi(value)) {
ALOGD("Silence is golden");
- setMasterMute(true);
+ setMasterMute_l(true);
}
}
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index 97103c4..674b047 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -710,17 +710,13 @@
virtual uint32_t latency() const;
- virtual status_t setMasterVolume(float value);
- virtual status_t setMasterMute(bool muted);
+ void setMasterVolume(float value);
+ void setMasterMute(bool muted);
- virtual float masterVolume() const { return mMasterVolume; }
- virtual bool masterMute() const { return mMasterMute; }
+ void setStreamVolume(audio_stream_type_t stream, float value);
+ void setStreamMute(audio_stream_type_t stream, bool muted);
- virtual status_t setStreamVolume(audio_stream_type_t stream, float value);
- virtual status_t setStreamMute(audio_stream_type_t stream, bool muted);
-
- virtual float streamVolume(audio_stream_type_t stream) const;
- virtual bool streamMute(audio_stream_type_t stream) const;
+ float streamVolume(audio_stream_type_t stream) const;
sp<Track> createTrack_l(
const sp<AudioFlinger::Client>& client,
@@ -776,6 +772,7 @@
int mBytesWritten;
private:
bool mMasterMute;
+ void setMasterMute_l(bool muted) { mMasterMute = muted; }
protected:
SortedVector< wp<Track> > mActiveTracks;
@@ -899,7 +896,11 @@
PlaybackThread *checkPlaybackThread_l(audio_io_handle_t output) const;
MixerThread *checkMixerThread_l(audio_io_handle_t output) const;
RecordThread *checkRecordThread_l(audio_io_handle_t input) const;
- float streamVolumeInternal(audio_stream_type_t stream) const
+ // no range check, AudioFlinger::mLock held
+ bool streamMute_l(audio_stream_type_t stream) const
+ { return mStreamTypes[stream].mute; }
+ // no range check, doesn't check per-thread stream volume, AudioFlinger::mLock held
+ float streamVolume_l(audio_stream_type_t stream) const
{ return mStreamTypes[stream].volume; }
void audioConfigChanged_l(int event, audio_io_handle_t ioHandle, void *param2);