commit 6637baae4244aec731c4014da72418d330636ae1
author Glenn Kasten <gkasten@google.com> Mon Jan 09 09:40:36 2012 -0800
committer Glenn Kasten <gkasten@google.com> Wed Feb 08 17:01:49 2012 -0800
tree 5244135379ecfdd8ce7394ccfed0b363ccabcf56
parent 98ec94c5854daccc3474758524e7f4adfe535ce0

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

services/audioflinger/AudioFlinger.cpp

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);
                         }
                     }