From: Jean-Michel Trivi <jmtrivi@google.com>
Date: Wed, 14 Feb 2018 18:48:51 +0000 (-0800)
Subject: AudioService: fix deadlock
X-Git-Tag: android-x86-9.0-r1~210^2~23^2
X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=085346acffb81159eb21f79a95d98f3200b8af1f;p=android-x86%2Fframeworks-base.git

AudioService: fix deadlock

Source of deadlock:
Thread A:
 readAudioSettings()
  -> checkAllAliasStreamVolumes()
    -> synchronized (VolumeStreamState.class)
         -> mStreamStates[streamType].setAllIndexes()
            -> synchronized (mSettingsLock)
Thread B:
 updateStreamVolumeAlias()
  -> mStreamStates[ACCESSIBILITY].setAllIndexes()
     -> synchronized (mSettingsLock)
        -> synchronized (VolumeStreamState.class)

Fix:
 Ensure all calls to VSS.setAllIndexes() are synchronized
 on mSettingsLock then on VolumeStreamState.class.

Bug: 72122435
Test: see bug for repro
Change-Id: I16ad1d1df88256291c36d8f5b8ebe830fe1b0b84
---

diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java
index fffe7dca7fc4..b3a6190569f6 100644
--- a/services/core/java/com/android/server/audio/AudioService.java
+++ b/services/core/java/com/android/server/audio/AudioService.java
@@ -1042,14 +1042,16 @@ public class AudioService extends IAudioService.Stub
     }
 
     private void checkAllAliasStreamVolumes() {
-        synchronized (VolumeStreamState.class) {
-            int numStreamTypes = AudioSystem.getNumStreamTypes();
-            for (int streamType = 0; streamType < numStreamTypes; streamType++) {
-                mStreamStates[streamType]
-                        .setAllIndexes(mStreamStates[mStreamVolumeAlias[streamType]], TAG);
-                // apply stream volume
-                if (!mStreamStates[streamType].mIsMuted) {
-                    mStreamStates[streamType].applyAllVolumes();
+        synchronized (mSettingsLock) {
+            synchronized (VolumeStreamState.class) {
+                int numStreamTypes = AudioSystem.getNumStreamTypes();
+                for (int streamType = 0; streamType < numStreamTypes; streamType++) {
+                    mStreamStates[streamType]
+                            .setAllIndexes(mStreamStates[mStreamVolumeAlias[streamType]], TAG);
+                    // apply stream volume
+                    if (!mStreamStates[streamType].mIsMuted) {
+                        mStreamStates[streamType].applyAllVolumes();
+                    }
                 }
             }
         }
@@ -1155,13 +1157,16 @@ public class AudioService extends IAudioService.Stub
         if (updateVolumes && mStreamStates != null) {
             updateDefaultVolumes();
 
-            mStreamStates[AudioSystem.STREAM_DTMF].setAllIndexes(mStreamStates[dtmfStreamAlias],
-                    caller);
-
-            mStreamStates[AudioSystem.STREAM_ACCESSIBILITY].mVolumeIndexSettingName =
-                    System.VOLUME_SETTINGS_INT[a11yStreamAlias];
-            mStreamStates[AudioSystem.STREAM_ACCESSIBILITY].setAllIndexes(
-                    mStreamStates[a11yStreamAlias], caller);
+            synchronized (mSettingsLock) {
+                synchronized (VolumeStreamState.class) {
+                    mStreamStates[AudioSystem.STREAM_DTMF]
+                            .setAllIndexes(mStreamStates[dtmfStreamAlias], caller);
+                    mStreamStates[AudioSystem.STREAM_ACCESSIBILITY].mVolumeIndexSettingName =
+                            System.VOLUME_SETTINGS_INT[a11yStreamAlias];
+                    mStreamStates[AudioSystem.STREAM_ACCESSIBILITY].setAllIndexes(
+                            mStreamStates[a11yStreamAlias], caller);
+                }
+            }
             if (sIndependentA11yVolume) {
                 // restore the a11y values from the settings
                 mStreamStates[AudioSystem.STREAM_ACCESSIBILITY].readSettings();
@@ -4604,39 +4609,36 @@ public class AudioService extends IAudioService.Stub
          * @param srcStream
          * @param caller
          */
+        // must be sync'd on mSettingsLock before VolumeStreamState.class
+        @GuardedBy("VolumeStreamState.class")
         public void setAllIndexes(VolumeStreamState srcStream, String caller) {
             if (mStreamType == srcStream.mStreamType) {
                 return;
             }
-            synchronized (mSettingsLock) {
-                synchronized (VolumeStreamState.class) {
-                    int srcStreamType = srcStream.getStreamType();
-                    // apply default device volume from source stream to all devices first in case
-                    // some devices are present in this stream state but not in source stream state
-                    int index = srcStream.getIndex(AudioSystem.DEVICE_OUT_DEFAULT);
-                    index = rescaleIndex(index, srcStreamType, mStreamType);
-                    for (int i = 0; i < mIndexMap.size(); i++) {
-                        mIndexMap.put(mIndexMap.keyAt(i), index);
-                    }
-                    // Now apply actual volume for devices in source stream state
-                    SparseIntArray srcMap = srcStream.mIndexMap;
-                    for (int i = 0; i < srcMap.size(); i++) {
-                        int device = srcMap.keyAt(i);
-                        index = srcMap.valueAt(i);
-                        index = rescaleIndex(index, srcStreamType, mStreamType);
-
-                        setIndex(index, device, caller);
-                    }
-                }
+            int srcStreamType = srcStream.getStreamType();
+            // apply default device volume from source stream to all devices first in case
+            // some devices are present in this stream state but not in source stream state
+            int index = srcStream.getIndex(AudioSystem.DEVICE_OUT_DEFAULT);
+            index = rescaleIndex(index, srcStreamType, mStreamType);
+            for (int i = 0; i < mIndexMap.size(); i++) {
+                mIndexMap.put(mIndexMap.keyAt(i), index);
+            }
+            // Now apply actual volume for devices in source stream state
+            SparseIntArray srcMap = srcStream.mIndexMap;
+            for (int i = 0; i < srcMap.size(); i++) {
+                int device = srcMap.keyAt(i);
+                index = srcMap.valueAt(i);
+                index = rescaleIndex(index, srcStreamType, mStreamType);
+
+                setIndex(index, device, caller);
             }
         }
 
-        @GuardedBy("mSettingsLock")
+        // must be sync'd on mSettingsLock before VolumeStreamState.class
+        @GuardedBy("VolumeStreamState.class")
         public void setAllIndexesToMax() {
-            synchronized (VolumeStreamState.class) {
-                for (int i = 0; i < mIndexMap.size(); i++) {
-                    mIndexMap.put(mIndexMap.keyAt(i), mIndexMax);
-                }
+            for (int i = 0; i < mIndexMap.size(); i++) {
+                mIndexMap.put(mIndexMap.keyAt(i), mIndexMax);
             }
         }
 
@@ -6224,15 +6226,17 @@ public class AudioService extends IAudioService.Stub
                 mCameraSoundForced = cameraSoundForced;
                 if (cameraSoundForcedChanged) {
                     if (!mIsSingleVolume) {
-                        VolumeStreamState s = mStreamStates[AudioSystem.STREAM_SYSTEM_ENFORCED];
-                        if (cameraSoundForced) {
-                            s.setAllIndexesToMax();
-                            mRingerModeAffectedStreams &=
-                                    ~(1 << AudioSystem.STREAM_SYSTEM_ENFORCED);
-                        } else {
-                            s.setAllIndexes(mStreamStates[AudioSystem.STREAM_SYSTEM], TAG);
-                            mRingerModeAffectedStreams |=
-                                    (1 << AudioSystem.STREAM_SYSTEM_ENFORCED);
+                        synchronized (VolumeStreamState.class) {
+                            VolumeStreamState s = mStreamStates[AudioSystem.STREAM_SYSTEM_ENFORCED];
+                            if (cameraSoundForced) {
+                                s.setAllIndexesToMax();
+                                mRingerModeAffectedStreams &=
+                                        ~(1 << AudioSystem.STREAM_SYSTEM_ENFORCED);
+                            } else {
+                                s.setAllIndexes(mStreamStates[AudioSystem.STREAM_SYSTEM], TAG);
+                                mRingerModeAffectedStreams |=
+                                        (1 << AudioSystem.STREAM_SYSTEM_ENFORCED);
+                            }
                         }
                         // take new state into account for streams muted by ringer mode
                         setRingerModeInt(getRingerModeInternal(), false);