Merge "AudioPlaybackConfiguration: prevent race condition on mIPlayerShell" into oc-mr1-dev

am: 982ee8e6c3

Change-Id: Ifa18821b2ae5b3103f5025df88c0e545bdca56af

media/java/android/media/AudioManager.java

@@ -3058,7 +3058,11 @@ public class AudioManager {
 
     private final IPlaybackConfigDispatcher mPlayCb = new IPlaybackConfigDispatcher.Stub() {
         @Override
-        public void dispatchPlaybackConfigChange(List<AudioPlaybackConfiguration> configs) {
+        public void dispatchPlaybackConfigChange(List<AudioPlaybackConfiguration> configs,
+                boolean flush) {
+            if (flush) {
+                Binder.flushPendingCommands();
+            }
             synchronized(mPlaybackCallbackLock) {
                 if (mPlaybackCallbackList != null) {
                     for (int i=0 ; i < mPlaybackCallbackList.size() ; i++) {

media/java/android/media/AudioPlaybackConfiguration.java

@@ -212,8 +212,10 @@ public final class AudioPlaybackConfiguration implements Parcelable {
      * @hide
      */
     public void init() {
-        if (mIPlayerShell != null) {
+        synchronized (this) {
-            mIPlayerShell.monitorDeath();
+            if (mIPlayerShell != null) {
+                mIPlayerShell.monitorDeath();
+            }
         }
     }
 
@@ -322,7 +324,11 @@ public final class AudioPlaybackConfiguration implements Parcelable {
      */
     @SystemApi
     public PlayerProxy getPlayerProxy() {
-        return mIPlayerShell == null ? null : new PlayerProxy(this);
+        final IPlayerShell ips;
+        synchronized (this) {
+            ips = mIPlayerShell;
+        }
+        return ips == null ? null : new PlayerProxy(this);
     }
 
     /**
@@ -330,7 +336,11 @@ public final class AudioPlaybackConfiguration implements Parcelable {
      * @return the IPlayer interface for the associated player
      */
     IPlayer getIPlayer() {
-        return mIPlayerShell == null ? null : mIPlayerShell.getIPlayer();
+        final IPlayerShell ips;
+        synchronized (this) {
+            ips = mIPlayerShell;
+        }
+        return ips == null ? null : ips.getIPlayer();
     }
 
     /**
@@ -351,10 +361,14 @@ public final class AudioPlaybackConfiguration implements Parcelable {
      * @return true if the state changed, false otherwise
      */
     public boolean handleStateEvent(int event) {
-        final boolean changed = (mPlayerState != event);
+        final boolean changed;
-        mPlayerState = event;
+        synchronized (this) {
-        if ((event == PLAYER_STATE_RELEASED) && (mIPlayerShell != null)) {
+            changed = (mPlayerState != event);
-            mIPlayerShell.release();
+            mPlayerState = event;
+            if (changed && (event == PLAYER_STATE_RELEASED) && (mIPlayerShell != null)) {
+                mIPlayerShell.release();
+                mIPlayerShell = null;
+            }
         }
         return changed;
     }
@@ -447,7 +461,11 @@ public final class AudioPlaybackConfiguration implements Parcelable {
         dest.writeInt(mClientPid);
         dest.writeInt(mPlayerState);
         mPlayerAttr.writeToParcel(dest, 0);
-        dest.writeStrongInterface(mIPlayerShell == null ? null : mIPlayerShell.getIPlayer());
+        final IPlayerShell ips;
+        synchronized (this) {
+            ips = mIPlayerShell;
+        }
+        dest.writeStrongInterface(ips == null ? null : ips.getIPlayer());
     }
 
     private AudioPlaybackConfiguration(Parcel in) {
@@ -479,14 +497,17 @@ public final class AudioPlaybackConfiguration implements Parcelable {
     static final class IPlayerShell implements IBinder.DeathRecipient {
 
         final AudioPlaybackConfiguration mMonitor; // never null
-        private IPlayer mIPlayer;
+        private volatile IPlayer mIPlayer;
 
         IPlayerShell(@NonNull AudioPlaybackConfiguration monitor, @NonNull IPlayer iplayer) {
             mMonitor = monitor;
             mIPlayer = iplayer;
         }
 
-        void monitorDeath() {
+        synchronized void monitorDeath() {
+            if (mIPlayer == null) {
+                return;
+            }
             try {
                 mIPlayer.asBinder().linkToDeath(this, 0);
             } catch (RemoteException e) {
@@ -509,8 +530,13 @@ public final class AudioPlaybackConfiguration implements Parcelable {
             } else if (DEBUG) { Log.i(TAG, "IPlayerShell binderDied"); }
         }
 
-        void release() {
+        synchronized void release() {
+            if (mIPlayer == null) {
+                return;
+            }
             mIPlayer.asBinder().unlinkToDeath(this, 0);
+            mIPlayer = null;
+            Binder.flushPendingCommands();
         }
     }
 
@@ -532,7 +558,7 @@ public final class AudioPlaybackConfiguration implements Parcelable {
             case PLAYER_TYPE_HW_SOURCE: return "hardware source";
             case PLAYER_TYPE_EXTERNAL_PROXY: return "external proxy";
             default:
-                return "unknown player type - FIXME";
+                return "unknown player type " + type + " - FIXME";
         }
     }
 

media/java/android/media/IPlaybackConfigDispatcher.aidl

@@ -25,6 +25,6 @@ import android.media.AudioPlaybackConfiguration;
  */
 oneway interface IPlaybackConfigDispatcher {
 
-    void dispatchPlaybackConfigChange(in List<AudioPlaybackConfiguration> configs);
+    void dispatchPlaybackConfigChange(in List<AudioPlaybackConfiguration> configs, in boolean flush);
 
 }

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

@@ -171,7 +171,7 @@ public final class PlaybackActivityMonitor
             }
         }
         if (change) {
-            dispatchPlaybackChange();
+            dispatchPlaybackChange(false);
         }
     }
 
@@ -210,7 +210,7 @@ public final class PlaybackActivityMonitor
             }
         }
         if (change) {
-            dispatchPlaybackChange();
+            dispatchPlaybackChange(event == AudioPlaybackConfiguration.PLAYER_STATE_RELEASED);
         }
     }
 
@@ -244,6 +244,14 @@ public final class PlaybackActivityMonitor
         pw.println("\nPlaybackActivityMonitor dump time: "
                 + DateFormat.getTimeInstance().format(new Date()));
         synchronized(mPlayerLock) {
+            pw.println("\n  playback listeners:");
+            synchronized(mClients) {
+                for (PlayMonitorClient pmc : mClients) {
+                    pw.print(" " + (pmc.mIsPrivileged ? "(S)" : "(P)")
+                            + pmc.toString());
+                }
+            }
+            pw.println("\n");
             // all players
             pw.println("\n  players:");
             final List<Integer> piidIntList = new ArrayList<Integer>(mPlayers.keySet());
@@ -268,7 +276,7 @@ public final class PlaybackActivityMonitor
             for (int uid : mBannedUids) {
                 pw.print(" " + uid);
             }
-            pw.println();
+            pw.println("\n");
             // log
             mEventLogger.dump(pw);
         }
@@ -293,7 +301,11 @@ public final class PlaybackActivityMonitor
         return true;
     }
 
-    private void dispatchPlaybackChange() {
+    /**
+     * Sends new list after update of playback configurations
+     * @param iplayerReleased indicates if the change was due to a player being released
+     */
+    private void dispatchPlaybackChange(boolean iplayerReleased) {
         synchronized (mClients) {
             // typical use case, nobody is listening, don't do any work
             if (mClients.isEmpty()) {
@@ -324,9 +336,12 @@ public final class PlaybackActivityMonitor
                     // do not spam the logs if there are problems communicating with this client
                     if (pmc.mErrorCount < PlayMonitorClient.MAX_ERRORS) {
                         if (pmc.mIsPrivileged) {
-                            pmc.mDispatcherCb.dispatchPlaybackConfigChange(configsSystem);
+                            pmc.mDispatcherCb.dispatchPlaybackConfigChange(configsSystem,
+                                    iplayerReleased);
                         } else {
-                            pmc.mDispatcherCb.dispatchPlaybackConfigChange(configsPublic);
+                            // non-system clients don't have the control interface IPlayer, so
+                            // they don't need to flush commands when a player was released
+                            pmc.mDispatcherCb.dispatchPlaybackConfigChange(configsPublic, false);
                         }
                     }
                 } catch (RemoteException e) {

services/core/java/com/android/server/media/AudioPlaybackMonitor.java

@@ -100,7 +100,11 @@ class AudioPlaybackMonitor extends IPlaybackConfigDispatcher.Stub {
      * @param configs List of the current audio playback configuration
      */
     @Override
-    public void dispatchPlaybackConfigChange(List<AudioPlaybackConfiguration> configs) {
+    public void dispatchPlaybackConfigChange(List<AudioPlaybackConfiguration> configs,
+            boolean flush) {
+        if (flush) {
+            Binder.flushPendingCommands();
+        }
         final long token = Binder.clearCallingIdentity();
         try {
             List<Integer> newActiveAudioPlaybackClientUids = new ArrayList<>();