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

Commit 776a3993 authored by Jean-Michel Trivi's avatar Jean-Michel Trivi
Browse files

AudioPlaybackConfiguration: prevent race condition on mIPlayerShell 

Synchronize changes to mIPlayerShell after release of corresponding
  player.
Flush binder commands when a player is released, in AudioService
  and in the clients that have an AudioPlaybackCallback implementation.
  Do the same in MediaSessionService, which directly implements
  the IPlaybackConfigDispatcher interface, without going through
  the AudioPlaybackCallback registration and notification
  mechanisms.

Test: adb shell /system/bin/write_sine_callback -m2 -pl
Bug: 65450109
Change-Id: I2f0697e0e164283284ce30d2cc736c4f8df270c4
parent f531f886

media/java/android/media/AudioManager.java

+5 −1
Original line number Diff line number Diff line
@@ -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

+39 −13
Original line number Diff line number Diff line
@@ -212,10 +212,12 @@ public final class AudioPlaybackConfiguration implements Parcelable { 
     * @hide

     */

    public void init() {

        synchronized (this) {

            if (mIPlayerShell != null) {

                mIPlayerShell.monitorDeath();

            }

        }

    }



    // Note that this method is called server side, so no "privileged" information is ever sent

    // to a client that is not supposed to have access to it.
@@ -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;

        synchronized (this) {

            changed = (mPlayerState != event);

            mPlayerState = event;

        if ((event == PLAYER_STATE_RELEASED) && (mIPlayerShell != null)) {

            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

+1 −1
Original line number Diff line number Diff line
@@ -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

+21 −6
Original line number Diff line number Diff line
@@ -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

+5 −1
Original line number Diff line number Diff line
@@ -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<>();