Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove calls to client while holding lock

Browse Source 
The dispatchPlaybackConfigChange callback could cause a deadlock in
case the client is running in the same process as the AudioService and
at the same time it's unregistering a client while a dispatch is active.

Test: CtsMediaAudioTestCases and dumpsys audio
Bug: 227596824
Change-Id: I6c6c13e254cb048aebc63cb594b0c4658cf86b91
This commit is contained in:
Vlad Popa
2022-09-06 14:25:46 +02:00
parent 7b46b29bce
commit b927ac9fdc
1 changed files with 85 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files

159
services/core/java/com/android/server/audio/PlaybackActivityMonitor.java
View File

@@ -49,6 +49,7 @@ import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.function.Consumer;
/**
@@ -104,11 +105,7 @@ public final class PlaybackActivityMonitor
private static final VolumeShaper.Operation PLAY_SKIP_RAMP =
new VolumeShaper.Operation.Builder(PLAY_CREATE_IF_NEEDED).setXOffset(1.0f).build();
private final ArrayList<PlayMonitorClient> mClients = new ArrayList<PlayMonitorClient>();
// a public client is one that needs an anonymized version of the playback configurations, we
// keep track of whether there is at least one to know when we need to create the list of
// playback configurations that do not contain uid/pid/package name information.
private boolean mHasPublicClients = false;
private final ConcurrentLinkedQueue<PlayMonitorClient> mClients = new ConcurrentLinkedQueue<>();
private final Object mPlayerLock = new Object();
@GuardedBy("mPlayerLock")
@@ -458,11 +455,9 @@ public final class PlaybackActivityMonitor
+ 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());
}
for (PlayMonitorClient pmc : mClients) {
pw.print(" " + (pmc.isPrivileged() ? "(S)" : "(P)")
+ pmc.toString());
}
pw.println("\n");
// all players
@@ -534,48 +529,33 @@ public final class PlaybackActivityMonitor
* @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()) {
return;
}
}
if (DEBUG) { Log.v(TAG, "dispatchPlaybackChange to " + mClients.size() + " clients"); }
final List<AudioPlaybackConfiguration> configsSystem;
// list of playback configurations for "public consumption". It is only computed if there
// list of playback configurations for "public consumption". It is computed lazy if there
// are non-system playback activity listeners.
final List<AudioPlaybackConfiguration> configsPublic;
List<AudioPlaybackConfiguration> configsPublic = null;
synchronized (mPlayerLock) {
if (mPlayers.isEmpty()) {
return;
}
configsSystem = new ArrayList<AudioPlaybackConfiguration>(mPlayers.values());
configsSystem = new ArrayList<>(mPlayers.values());
}
synchronized (mClients) {
// was done at beginning of method, but could have changed
if (mClients.isEmpty()) {
return;
}
configsPublic = mHasPublicClients ? anonymizeForPublicConsumption(configsSystem) : null;
final Iterator<PlayMonitorClient> clientIterator = mClients.iterator();
while (clientIterator.hasNext()) {
final PlayMonitorClient pmc = clientIterator.next();
try {
// 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,
iplayerReleased);
} else {
// 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);
}
final Iterator<PlayMonitorClient> clientIterator = mClients.iterator();
while (clientIterator.hasNext()) {
final PlayMonitorClient pmc = clientIterator.next();
// do not spam the logs if there are problems communicating with this client
if (!pmc.reachedMaxErrorCount()) {
if (pmc.isPrivileged()) {
pmc.dispatchPlaybackConfigChange(configsSystem,
iplayerReleased);
} else {
if (configsPublic == null) {
configsPublic = anonymizeForPublicConsumption(configsSystem);
}
} catch (RemoteException e) {
pmc.mErrorCount++;
Log.e(TAG, "Error (" + pmc.mErrorCount +
") trying to dispatch playback config change to " + pmc, e);
// non-system clients don't have the control interface IPlayer, so
// they don't need to flush commands when a player was released
pmc.dispatchPlaybackConfigChange(configsPublic, false);
}
}
}
@@ -798,14 +778,9 @@ public final class PlaybackActivityMonitor
if (pcdb == null) {
return;
}
synchronized(mClients) {
final PlayMonitorClient pmc = new PlayMonitorClient(pcdb, isPrivileged);
if (pmc.init()) {
if (!isPrivileged) {
mHasPublicClients = true;
}
mClients.add(pmc);
}
final PlayMonitorClient pmc = new PlayMonitorClient(pcdb, isPrivileged);
if (pmc.init()) {
mClients.add(pmc);
}
}
@@ -813,23 +788,14 @@ public final class PlaybackActivityMonitor
if (pcdb == null) {
return;
}
synchronized(mClients) {
final Iterator<PlayMonitorClient> clientIterator = mClients.iterator();
boolean hasPublicClients = false;
// iterate over the clients to remove the dispatcher to remove, and reevaluate at
// the same time if we still have a public client.
while (clientIterator.hasNext()) {
PlayMonitorClient pmc = clientIterator.next();
if (pcdb.asBinder().equals(pmc.mDispatcherCb.asBinder())) {
pmc.release();
clientIterator.remove();
} else {
if (!pmc.mIsPrivileged) {
hasPublicClients = true;
}
}
final Iterator<PlayMonitorClient> clientIterator = mClients.iterator();
// iterate over the clients to remove the dispatcher
while (clientIterator.hasNext()) {
PlayMonitorClient pmc = clientIterator.next();
if (pmc.equalsDispatcher(pcdb)) {
pmc.release();
clientIterator.remove();
}
mHasPublicClients = hasPublicClients;
}
}
@@ -857,24 +823,34 @@ public final class PlaybackActivityMonitor
// can afford to be static because only one PlaybackActivityMonitor ever instantiated
static PlaybackActivityMonitor sListenerDeathMonitor;
final IPlaybackConfigDispatcher mDispatcherCb;
final boolean mIsPrivileged;
int mErrorCount = 0;
// number of errors after which we don't update this client anymore to not spam the logs
static final int MAX_ERRORS = 5;
private static final int MAX_ERRORS = 5;
private final IPlaybackConfigDispatcher mDispatcherCb;
@GuardedBy("this")
private final boolean mIsPrivileged;
@GuardedBy("this")
private boolean mIsReleased = false;
@GuardedBy("this")
private int mErrorCount = 0;
PlayMonitorClient(IPlaybackConfigDispatcher pcdb, boolean isPrivileged) {
mDispatcherCb = pcdb;
mIsPrivileged = isPrivileged;
}
@Override
public void binderDied() {
Log.w(TAG, "client died");
sListenerDeathMonitor.unregisterPlaybackCallback(mDispatcherCb);
}
boolean init() {
synchronized boolean init() {
if (mIsReleased) {
// Do not init after release
return false;
}
try {
mDispatcherCb.asBinder().linkToDeath(this, 0);
return true;
@@ -884,8 +860,43 @@ public final class PlaybackActivityMonitor
}
}
void release() {
synchronized void release() {
mDispatcherCb.asBinder().unlinkToDeath(this, 0);
mIsReleased = true;
}
void dispatchPlaybackConfigChange(List<AudioPlaybackConfiguration> configs,
boolean flush) {
synchronized (this) {
if (mIsReleased) {
// Do not dispatch anything after release
return;
}
}
try {
mDispatcherCb.dispatchPlaybackConfigChange(configs, flush);
} catch (RemoteException e) {
synchronized (this) {
mErrorCount++;
Log.e(TAG, "Error (" + mErrorCount
+ ") trying to dispatch playback config change to " + this, e);
}
}
}
synchronized boolean isPrivileged() {
return mIsPrivileged;
}
synchronized boolean reachedMaxErrorCount() {
return mErrorCount >= MAX_ERRORS;
}
synchronized boolean equalsDispatcher(IPlaybackConfigDispatcher pcdb) {
if (pcdb == null) {
return false;
}
return pcdb.asBinder().equals(mDispatcherCb.asBinder());
}
}