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

This commit is contained in:
Jean-Michel Trivi
2017-09-15 01:37:55 +00:00
committed by Android (Google) Code Review
5 changed files with 71 additions and 22 deletions

View File

@@ -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++) {

View File

@@ -212,8 +212,10 @@ public final class AudioPlaybackConfiguration implements Parcelable {
* @hide
*/
public void init() {
if (mIPlayerShell != null) {
mIPlayerShell.monitorDeath();
synchronized (this) {
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);
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 @@ 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";
}
}

View File

@@ -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);
}

View File

@@ -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) {

View File

@@ -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<>();