Merge "Fix deadlock caused by wrong locking order" into udc-dev am: cbde6a8f33 am: 3cfa4ec896

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/23681582

Change-Id: Ieb1bdb1a86e43b6c2b72b86823dff5b65da92d9b
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

1 files changed, 127 insertions, 95 deletions

diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java
index c177c4efb5ef..53ed38edffe4 100644
--- a/services/core/java/com/android/server/audio/AudioService.java
+++ b/services/core/java/com/android/server/audio/AudioService.java
@@ -3666,25 +3666,29 @@ public class AudioService extends IAudioService.Stub
      * and aliases before mute change changed and after.
      */
     private void muteAliasStreams(int streamAlias, boolean state) {
-        synchronized (VolumeStreamState.class) {
-            List<Integer> streamsToMute = new ArrayList<>();
-            for (int stream = 0; stream < mStreamStates.length; stream++) {
-                VolumeStreamState vss = mStreamStates[stream];
-                if (streamAlias == mStreamVolumeAlias[stream] && vss.isMutable()) {
-                    if (!(mCameraSoundForced
-                            && (vss.getStreamType()
-                                    == AudioSystem.STREAM_SYSTEM_ENFORCED))) {
-                        boolean changed = vss.mute(state, /* apply= */ false, "muteAliasStreams");
-                        if (changed) {
-                            streamsToMute.add(stream);
+        // Locking mSettingsLock to avoid inversion when calling doMute -> updateVolumeGroupIndex
+        synchronized (mSettingsLock) {
+            synchronized (VolumeStreamState.class) {
+                List<Integer> streamsToMute = new ArrayList<>();
+                for (int stream = 0; stream < mStreamStates.length; stream++) {
+                    VolumeStreamState vss = mStreamStates[stream];
+                    if (streamAlias == mStreamVolumeAlias[stream] && vss.isMutable()) {
+                        if (!(mCameraSoundForced
+                                && (vss.getStreamType()
+                                == AudioSystem.STREAM_SYSTEM_ENFORCED))) {
+                            boolean changed = vss.mute(state, /* apply= */ false,
+                                    "muteAliasStreams");
+                            if (changed) {
+                                streamsToMute.add(stream);
+                            }
                         }
                     }
                 }
+                streamsToMute.forEach(streamToMute -> {
+                    mStreamStates[streamToMute].doMute();
+                    broadcastMuteSetting(streamToMute, state);
+                });
             }
-            streamsToMute.forEach(streamToMute -> {
-                mStreamStates[streamToMute].doMute();
-                broadcastMuteSetting(streamToMute, state);
-            });
         }
     }
 
@@ -3699,18 +3703,22 @@ public class AudioService extends IAudioService.Stub
     // Called after a delay when volume down is pressed while muted
     private void onUnmuteStream(int stream, int flags) {
         boolean wasMuted;
-        synchronized (VolumeStreamState.class) {
-            final VolumeStreamState streamState = mStreamStates[stream];
-            // if unmuting causes a change, it was muted
-            wasMuted = streamState.mute(false, "onUnmuteStream");
+        // Locking mSettingsLock to avoid inversion when calling vss.mute -> vss.doMute ->
+        // vss.updateVolumeGroupIndex
+        synchronized (mSettingsLock) {
+            synchronized (VolumeStreamState.class) {
+                final VolumeStreamState streamState = mStreamStates[stream];
+                // if unmuting causes a change, it was muted
+                wasMuted = streamState.mute(false, "onUnmuteStream");
 
-            final int device = getDeviceForStream(stream);
-            final int index = streamState.getIndex(device);
-            sendVolumeUpdate(stream, index, index, flags, device);
-        }
-        if (stream == AudioSystem.STREAM_MUSIC && wasMuted) {
-            synchronized (mHdmiClientLock) {
-                maybeSendSystemAudioStatusCommand(true);
+                final int device = getDeviceForStream(stream);
+                final int index = streamState.getIndex(device);
+                sendVolumeUpdate(stream, index, index, flags, device);
+            }
+            if (stream == AudioSystem.STREAM_MUSIC && wasMuted) {
+                synchronized (mHdmiClientLock) {
+                    maybeSendSystemAudioStatusCommand(true);
+                }
             }
         }
     }
@@ -7611,7 +7619,11 @@ public class AudioService extends IAudioService.Stub
                 Log.i(TAG, String.format("onAccessoryPlugMediaUnmute unmuting device=%d [%s]",
                         newDevice, AudioSystem.getOutputDeviceName(newDevice)));
             }
-            mStreamStates[AudioSystem.STREAM_MUSIC].mute(false, "onAccessoryPlugMediaUnmute");
+            // Locking mSettingsLock to avoid inversion when calling vss.mute -> vss.doMute ->
+            // vss.updateVolumeGroupIndex
+            synchronized (mSettingsLock) {
+                mStreamStates[AudioSystem.STREAM_MUSIC].mute(false, "onAccessoryPlugMediaUnmute");
+            }
         }
     }
 
@@ -7646,9 +7658,14 @@ public class AudioService extends IAudioService.Stub
                 continue;
             }
         }
-        for (int i = 0; i < sVolumeGroupStates.size(); i++) {
-            final VolumeGroupState vgs = sVolumeGroupStates.valueAt(i);
-            vgs.applyAllVolumes(/* userSwitch= */ false);
+
+        // need mSettingsLock for vgs.applyAllVolumes -> vss.setIndex which grabs this lock after
+        // VSS.class. Locking order needs to be preserved
+        synchronized (mSettingsLock) {
+            for (int i = 0; i < sVolumeGroupStates.size(); i++) {
+                final VolumeGroupState vgs = sVolumeGroupStates.valueAt(i);
+                vgs.applyAllVolumes(/* userSwitch= */ false);
+            }
         }
     }
 
@@ -7685,9 +7702,14 @@ public class AudioService extends IAudioService.Stub
         if (DEBUG_VOL) {
             Log.v(TAG, "restoreVolumeGroups");
         }
-        for (int i = 0; i < sVolumeGroupStates.size(); i++) {
-            final VolumeGroupState vgs = sVolumeGroupStates.valueAt(i);
-            vgs.applyAllVolumes(false/*userSwitch*/);
+
+        // need mSettingsLock for vgs.applyAllVolumes -> vss.setIndex which grabs this lock after
+        // VSS.class. Locking order needs to be preserved
+        synchronized (mSettingsLock) {
+            for (int i = 0; i < sVolumeGroupStates.size(); i++) {
+                final VolumeGroupState vgs = sVolumeGroupStates.valueAt(i);
+                vgs.applyAllVolumes(false/*userSwitch*/);
+            }
         }
     }
 
@@ -7824,49 +7846,51 @@ public class AudioService extends IAudioService.Stub
         }
 
         public void adjustVolume(int direction, int flags) {
-            synchronized (AudioService.VolumeStreamState.class) {
-                int device = getDeviceForVolume();
-                int previousIndex = getIndex(device);
-                if (isMuteAdjust(direction) && !isMutable()) {
-                    // Non mutable volume group
-                    if (DEBUG_VOL) {
-                        Log.d(TAG, "invalid mute on unmutable volume group " + name());
-                    }
-                    return;
-                }
-                switch (direction) {
-                    case AudioManager.ADJUST_TOGGLE_MUTE: {
-                        // Note: If muted by volume 0, unmute will restore volume 0.
-                        mute(!mIsMuted);
-                        break;
+            synchronized (mSettingsLock) {
+                synchronized (AudioService.VolumeStreamState.class) {
+                    int device = getDeviceForVolume();
+                    int previousIndex = getIndex(device);
+                    if (isMuteAdjust(direction) && !isMutable()) {
+                        // Non mutable volume group
+                        if (DEBUG_VOL) {
+                            Log.d(TAG, "invalid mute on unmutable volume group " + name());
+                        }
+                        return;
                     }
-                    case AudioManager.ADJUST_UNMUTE:
-                        // Note: If muted by volume 0, unmute will restore volume 0.
-                        mute(false);
-                        break;
-                    case AudioManager.ADJUST_MUTE:
-                        // May be already muted by setvolume 0, prevent from setting same value
-                        if (previousIndex != 0) {
-                            // bypass persist
-                            mute(true);
+                    switch (direction) {
+                        case AudioManager.ADJUST_TOGGLE_MUTE: {
+                            // Note: If muted by volume 0, unmute will restore volume 0.
+                            mute(!mIsMuted);
+                            break;
                         }
-                        mIsMuted = true;
-                        break;
-                    case AudioManager.ADJUST_RAISE:
-                        // As for stream, RAISE during mute will increment the index
-                        setVolumeIndex(Math.min(previousIndex + 1, mIndexMax),  device, flags);
-                        break;
-                    case AudioManager.ADJUST_LOWER:
-                        // For stream, ADJUST_LOWER on a muted VSS is a no-op
-                        // If we decide to unmute on ADJUST_LOWER, cannot fallback on
-                        // adjustStreamVolume for group associated to legacy stream type
-                        if (isMuted() && previousIndex != 0) {
+                        case AudioManager.ADJUST_UNMUTE:
+                            // Note: If muted by volume 0, unmute will restore volume 0.
                             mute(false);
-                        } else {
-                            int newIndex = Math.max(previousIndex - 1, mIndexMin);
-                            setVolumeIndex(newIndex, device, flags);
-                        }
-                        break;
+                            break;
+                        case AudioManager.ADJUST_MUTE:
+                            // May be already muted by setvolume 0, prevent from setting same value
+                            if (previousIndex != 0) {
+                                // bypass persist
+                                mute(true);
+                            }
+                            mIsMuted = true;
+                            break;
+                        case AudioManager.ADJUST_RAISE:
+                            // As for stream, RAISE during mute will increment the index
+                            setVolumeIndex(Math.min(previousIndex + 1, mIndexMax),  device, flags);
+                            break;
+                        case AudioManager.ADJUST_LOWER:
+                            // For stream, ADJUST_LOWER on a muted VSS is a no-op
+                            // If we decide to unmute on ADJUST_LOWER, cannot fallback on
+                            // adjustStreamVolume for group associated to legacy stream type
+                            if (isMuted() && previousIndex != 0) {
+                                mute(false);
+                            } else {
+                                int newIndex = Math.max(previousIndex - 1, mIndexMin);
+                                setVolumeIndex(newIndex, device, flags);
+                            }
+                            break;
+                    }
                 }
             }
         }
@@ -7878,11 +7902,13 @@ public class AudioService extends IAudioService.Stub
         }
 
         public void setVolumeIndex(int index, int flags) {
-            synchronized (AudioService.VolumeStreamState.class) {
-                if (mUseFixedVolume) {
-                    return;
+            synchronized (mSettingsLock) {
+                synchronized (AudioService.VolumeStreamState.class) {
+                    if (mUseFixedVolume) {
+                        return;
+                    }
+                    setVolumeIndex(index, getDeviceForVolume(), flags);
                 }
-                setVolumeIndex(index, getDeviceForVolume(), flags);
             }
         }
 
@@ -8689,24 +8715,30 @@ public class AudioService extends IAudioService.Stub
 
         // If associated to volume group, update group cache
         private void updateVolumeGroupIndex(int device, boolean forceMuteState) {
-            synchronized (VolumeStreamState.class) {
-                if (mVolumeGroupState != null) {
-                    int groupIndex = (getIndex(device) + 5) / 10;
-                    if (DEBUG_VOL) {
-                        Log.d(TAG, "updateVolumeGroupIndex for stream " + mStreamType
-                                + ", muted=" + mIsMuted + ", device=" + device + ", index="
-                                + getIndex(device) + ", group " + mVolumeGroupState.name()
-                                + " Muted=" + mVolumeGroupState.isMuted() + ", Index=" + groupIndex
-                                + ", forceMuteState=" + forceMuteState);
-                    }
-                    mVolumeGroupState.updateVolumeIndex(groupIndex, device);
-                    // Only propage mute of stream when applicable
-                    if (isMutable()) {
-                        // For call stream, align mute only when muted, not when index is set to 0
-                        mVolumeGroupState.mute(
-                                forceMuteState ? mIsMuted :
-                                        (groupIndex == 0 && !isCallStream(mStreamType))
-                                                || mIsMuted);
+            // need mSettingsLock when called from setIndex for vgs.mute -> vgs.applyAllVolumes ->
+            // vss.setIndex which grabs this lock after VSS.class. Locking order needs to be
+            // preserved
+            synchronized (mSettingsLock) {
+                synchronized (VolumeStreamState.class) {
+                    if (mVolumeGroupState != null) {
+                        int groupIndex = (getIndex(device) + 5) / 10;
+                        if (DEBUG_VOL) {
+                            Log.d(TAG, "updateVolumeGroupIndex for stream " + mStreamType
+                                    + ", muted=" + mIsMuted + ", device=" + device + ", index="
+                                    + getIndex(device) + ", group " + mVolumeGroupState.name()
+                                    + " Muted=" + mVolumeGroupState.isMuted() + ", Index="
+                                    + groupIndex + ", forceMuteState=" + forceMuteState);
+                        }
+                        mVolumeGroupState.updateVolumeIndex(groupIndex, device);
+                        // Only propage mute of stream when applicable
+                        if (isMutable()) {
+                            // For call stream, align mute only when muted, not when index is set to
+                            // 0
+                            mVolumeGroupState.mute(
+                                    forceMuteState ? mIsMuted :
+                                            (groupIndex == 0 && !isCallStream(mStreamType))
+                                                    || mIsMuted);
+                        }
                     }
                 }
             }