Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 71535717 authored by Vlad Popa's avatar Vlad Popa
Browse files

Remove calls to client while holding lock

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: Ibe2ee8a9161774f290f222ccf9d5bf9e9e4e1cfd
Merged-In: I6c6c13e254cb048aebc63cb594b0c4658cf86b91
parent 05d1acf6
Loading
Loading
Loading
Loading
+85 −74
Original line number Diff line number Diff line
@@ -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,12 +455,10 @@ 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)")
                pw.print(" " + (pmc.isPrivileged() ? "(S)" : "(P)")
                        + pmc.toString());
            }
            }
            pw.println("\n");
            // all players
            pw.println("\n  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,
            if (!pmc.reachedMaxErrorCount()) {
                if (pmc.isPrivileged()) {
                    pmc.dispatchPlaybackConfigChange(configsSystem,
                            iplayerReleased);
                } else {
                    if (configsPublic == null) {
                        configsPublic = anonymizeForPublicConsumption(configsSystem);
                    }
                    // 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) {
                    pmc.mErrorCount++;
                    Log.e(TAG, "Error (" + pmc.mErrorCount +
                            ") trying to dispatch playback config change to " + pmc, e);
                    pmc.dispatchPlaybackConfigChange(configsPublic, false);
                }
            }
        }
@@ -798,40 +778,26 @@ 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);
        }
    }
    }

    void unregisterPlaybackCallback(IPlaybackConfigDispatcher pcdb) {
        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.
        // iterate over the clients to remove the dispatcher
        while (clientIterator.hasNext()) {
            PlayMonitorClient pmc = clientIterator.next();
                if (pcdb.asBinder().equals(pmc.mDispatcherCb.asBinder())) {
            if (pmc.equalsDispatcher(pcdb)) {
                pmc.release();
                clientIterator.remove();
                } else {
                    if (!pmc.mIsPrivileged) {
                        hasPublicClients = true;
            }
        }
    }
            mHasPublicClients = hasPublicClients;
        }
    }

    List<AudioPlaybackConfiguration> getActivePlaybackConfigurations(boolean isPrivileged) {
        synchronized(mPlayers) {
@@ -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());
        }
    }