OSDN Git Service

Fixed unsafe lock upon safe media volume
authorhyomin.oh <hyomin.oh@samsung.com>
Thu, 18 Oct 2018 04:58:27 +0000 (13:58 +0900)
committerhyomin.oh <hyomin.oh@samsung.com>
Fri, 19 Oct 2018 03:39:46 +0000 (12:39 +0900)
Safe media volume uses lock as Integer.
But it is not safe since the Integer value is changed.

disableSafeMediaVolume()
   synchronized (mSafeMediaVolumeState)  // step. 1
      setSafeMediaVolumeEnabled()
         mSafeMediaVolumeState = SAFE_MEDIA_VOLUME_ACTIVE  // step. 2
      ...
      onSetStreamVolume(mPendingVolumeCommand.mStreamType, // step.4
      -> mPendingVolumeCommand is set as null by step.3
      -> it causes NPE and reboot

------------

setStreamVolume()
   synchronized (mSafeMediaVolumeState)
   -> mSafeMediaVolumeState was changed by step.2
   -> so that it would go next step
      mPendingVolumeCommand = null;  // step. 3

Test: Build Pass, manual test, change volume
Change-Id: I33f473d42ccbf0f9b177c6886622ecc2f8020f8d

services/core/java/com/android/server/audio/AudioService.java

index d056523..262e389 100644 (file)
@@ -819,9 +819,9 @@ public class AudioService extends IAudioService.Stub
                 new String("AudioService ctor"),
                 0);
 
-        mSafeMediaVolumeState = new Integer(Settings.Global.getInt(mContentResolver,
-                                                        Settings.Global.AUDIO_SAFE_VOLUME_STATE,
-                                                        SAFE_MEDIA_VOLUME_NOT_CONFIGURED));
+        mSafeMediaVolumeState = Settings.Global.getInt(mContentResolver,
+                                            Settings.Global.AUDIO_SAFE_VOLUME_STATE,
+                                            SAFE_MEDIA_VOLUME_NOT_CONFIGURED);
         // The default safe volume index read here will be replaced by the actual value when
         // the mcc is read by onConfigureSafeVolume()
         mSafeMediaVolumeIndex = mContext.getResources().getInteger(
@@ -1658,7 +1658,7 @@ public class AudioService extends IAudioService.Stub
         }
 
         // reset any pending volume command
-        synchronized (mSafeMediaVolumeState) {
+        synchronized (mSafeMediaVolumeStateLock) {
             mPendingVolumeCommand = null;
         }
 
@@ -2011,7 +2011,7 @@ public class AudioService extends IAudioService.Stub
             return;
         }
 
-        synchronized (mSafeMediaVolumeState) {
+        synchronized (mSafeMediaVolumeStateLock) {
             // reset any pending volume command
             mPendingVolumeCommand = null;
 
@@ -3238,7 +3238,7 @@ public class AudioService extends IAudioService.Stub
         checkAllAliasStreamVolumes();
         checkMuteAffectedStreams();
 
-        synchronized (mSafeMediaVolumeState) {
+        synchronized (mSafeMediaVolumeStateLock) {
             mMusicActiveMs = MathUtils.constrain(Settings.Secure.getIntForUser(mContentResolver,
                     Settings.Secure.UNSAFE_VOLUME_MUSIC_ACTIVE_MS, 0, UserHandle.USER_CURRENT),
                     0, UNSAFE_VOLUME_MUSIC_ACTIVE_MS_MAX);
@@ -4037,7 +4037,7 @@ public class AudioService extends IAudioService.Stub
     }
 
     private void onCheckMusicActive(String caller) {
-        synchronized (mSafeMediaVolumeState) {
+        synchronized (mSafeMediaVolumeStateLock) {
             if (mSafeMediaVolumeState == SAFE_MEDIA_VOLUME_INACTIVE) {
                 int device = getDeviceForStream(AudioSystem.STREAM_MUSIC);
 
@@ -4098,7 +4098,7 @@ public class AudioService extends IAudioService.Stub
     }
 
     private void onConfigureSafeVolume(boolean force, String caller) {
-        synchronized (mSafeMediaVolumeState) {
+        synchronized (mSafeMediaVolumeStateLock) {
             int mcc = mContext.getResources().getConfiguration().mcc;
             if ((mMcc != mcc) || ((mMcc == 0) && force)) {
                 mSafeMediaVolumeIndex = mContext.getResources().getInteger(
@@ -6952,7 +6952,8 @@ public class AudioService extends IAudioService.Stub
     private static final int SAFE_MEDIA_VOLUME_DISABLED = 1;
     private static final int SAFE_MEDIA_VOLUME_INACTIVE = 2;  // confirmed
     private static final int SAFE_MEDIA_VOLUME_ACTIVE = 3;  // unconfirmed
-    private Integer mSafeMediaVolumeState;
+    private int mSafeMediaVolumeState;
+    private final Object mSafeMediaVolumeStateLock = new Object();
 
     private int mMcc = 0;
     // mSafeMediaVolumeIndex is the cached value of config_safe_media_volume_index property
@@ -6992,7 +6993,7 @@ public class AudioService extends IAudioService.Stub
     }
 
     private void setSafeMediaVolumeEnabled(boolean on, String caller) {
-        synchronized (mSafeMediaVolumeState) {
+        synchronized (mSafeMediaVolumeStateLock) {
             if ((mSafeMediaVolumeState != SAFE_MEDIA_VOLUME_NOT_CONFIGURED) &&
                     (mSafeMediaVolumeState != SAFE_MEDIA_VOLUME_DISABLED)) {
                 if (on && (mSafeMediaVolumeState == SAFE_MEDIA_VOLUME_INACTIVE)) {
@@ -7040,7 +7041,7 @@ public class AudioService extends IAudioService.Stub
     }
 
     private boolean checkSafeMediaVolume(int streamType, int index, int device) {
-        synchronized (mSafeMediaVolumeState) {
+        synchronized (mSafeMediaVolumeStateLock) {
             if ((mSafeMediaVolumeState == SAFE_MEDIA_VOLUME_ACTIVE) &&
                     (mStreamVolumeAlias[streamType] == AudioSystem.STREAM_MUSIC) &&
                     ((device & mSafeMediaVolumeDevices) != 0) &&
@@ -7054,7 +7055,7 @@ public class AudioService extends IAudioService.Stub
     @Override
     public void disableSafeMediaVolume(String callingPackage) {
         enforceVolumeController("disable the safe media volume");
-        synchronized (mSafeMediaVolumeState) {
+        synchronized (mSafeMediaVolumeStateLock) {
             setSafeMediaVolumeEnabled(false, callingPackage);
             if (mPendingVolumeCommand != null) {
                 onSetStreamVolume(mPendingVolumeCommand.mStreamType,
@@ -7330,7 +7331,7 @@ public class AudioService extends IAudioService.Stub
         mVolumeLogger.dump(pw);
     }
 
-    private static String safeMediaVolumeStateToString(Integer state) {
+    private static String safeMediaVolumeStateToString(int state) {
         switch(state) {
             case SAFE_MEDIA_VOLUME_NOT_CONFIGURED: return "SAFE_MEDIA_VOLUME_NOT_CONFIGURED";
             case SAFE_MEDIA_VOLUME_DISABLED: return "SAFE_MEDIA_VOLUME_DISABLED";