Merge "AudioPlaybackConfiguration: prevent race condition on mIPlayerShell" into oc-mr1-dev
diff --git a/media/java/android/media/AudioManager.java b/media/java/android/media/AudioManager.java
index eea4628..3df1706 100644
--- a/media/java/android/media/AudioManager.java
+++ b/media/java/android/media/AudioManager.java
@@ -3058,7 +3058,11 @@
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++) {
diff --git a/media/java/android/media/AudioPlaybackConfiguration.java b/media/java/android/media/AudioPlaybackConfiguration.java
index 14bc555..8a36f91 100644
--- a/media/java/android/media/AudioPlaybackConfiguration.java
+++ b/media/java/android/media/AudioPlaybackConfiguration.java
@@ -212,8 +212,10 @@
* @hide
*/
public void init() {
- if (mIPlayerShell != null) {
- mIPlayerShell.monitorDeath();
+ synchronized (this) {
+ if (mIPlayerShell != null) {
+ mIPlayerShell.monitorDeath();
+ }
}
}
@@ -322,7 +324,11 @@
*/
@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 @@
* @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 @@
* @return true if the state changed, false otherwise
*/
public boolean handleStateEvent(int event) {
- final boolean changed = (mPlayerState != event);
- mPlayerState = event;
- if ((event == PLAYER_STATE_RELEASED) && (mIPlayerShell != null)) {
- mIPlayerShell.release();
+ final boolean changed;
+ synchronized (this) {
+ changed = (mPlayerState != event);
+ mPlayerState = event;
+ if (changed && (event == PLAYER_STATE_RELEASED) && (mIPlayerShell != null)) {
+ mIPlayerShell.release();
+ mIPlayerShell = null;
+ }
}
return changed;
}
@@ -447,7 +461,11 @@
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 @@
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 @@
} 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 @@
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";
}
}
diff --git a/media/java/android/media/IPlaybackConfigDispatcher.aidl b/media/java/android/media/IPlaybackConfigDispatcher.aidl
index 3cb5216..150ff26 100644
--- a/media/java/android/media/IPlaybackConfigDispatcher.aidl
+++ b/media/java/android/media/IPlaybackConfigDispatcher.aidl
@@ -25,6 +25,6 @@
*/
oneway interface IPlaybackConfigDispatcher {
- void dispatchPlaybackConfigChange(in List<AudioPlaybackConfiguration> configs);
+ void dispatchPlaybackConfigChange(in List<AudioPlaybackConfiguration> configs, in boolean flush);
}
diff --git a/services/core/java/com/android/server/audio/PlaybackActivityMonitor.java b/services/core/java/com/android/server/audio/PlaybackActivityMonitor.java
index 27e4d14..d1a37af 100644
--- a/services/core/java/com/android/server/audio/PlaybackActivityMonitor.java
+++ b/services/core/java/com/android/server/audio/PlaybackActivityMonitor.java
@@ -171,7 +171,7 @@
}
}
if (change) {
- dispatchPlaybackChange();
+ dispatchPlaybackChange(false);
}
}
@@ -210,7 +210,7 @@
}
}
if (change) {
- dispatchPlaybackChange();
+ dispatchPlaybackChange(event == AudioPlaybackConfiguration.PLAYER_STATE_RELEASED);
}
}
@@ -244,6 +244,14 @@
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 @@
for (int uid : mBannedUids) {
pw.print(" " + uid);
}
- pw.println();
+ pw.println("\n");
// log
mEventLogger.dump(pw);
}
@@ -293,7 +301,11 @@
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 @@
// 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) {
diff --git a/services/core/java/com/android/server/media/AudioPlaybackMonitor.java b/services/core/java/com/android/server/media/AudioPlaybackMonitor.java
index f6f7676..791ee82 100644
--- a/services/core/java/com/android/server/media/AudioPlaybackMonitor.java
+++ b/services/core/java/com/android/server/media/AudioPlaybackMonitor.java
@@ -100,7 +100,11 @@
* @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<>();