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

Commit 89f88ae9 authored by Alex Shabalin's avatar Alex Shabalin
Browse files

Prevent invoking callbacks from `synchronized` block. 

The callback implementations might call InfoMediaManager getter calls
resulting in a deadlock.

Test: atest InfoMediaManagerTest, on a device.
Flag: com.android.media.flags.enable_suggested_device_api
Fix: 422720290
Change-Id: Ie41df25780f17bc0b8acd2a4e32be94c3bd08573
parent a747a967

packages/SettingsLib/src/com/android/settingslib/media/InfoMediaManager.java

+59 −40
Original line number Diff line number Diff line
@@ -372,6 +372,7 @@ public abstract class InfoMediaManager { 
        synchronized (mLock) {

            buildAvailableRoutes();

        }

        updateDeviceSuggestion();

    }



    protected final void notifyCurrentConnectedDeviceChanged() {
@@ -698,7 +699,7 @@ public abstract class InfoMediaManager { 
                || sessionInfo.getVolumeHandling() != MediaRoute2Info.PLAYBACK_VOLUME_FIXED;

    }



    protected void updateDeviceSuggestion(

    protected void notifyDeviceSuggestionUpdated(

            String suggestingPackageName, @Nullable List<SuggestedDeviceInfo> suggestions) {

        if (suggestions == null) {

            mSuggestedDeviceMap.remove(suggestingPackageName);
@@ -708,10 +709,22 @@ public abstract class InfoMediaManager { 
        updateDeviceSuggestion();

    }



    protected final void updateDeviceSuggestion() {

    private void updateDeviceSuggestion() {

        if (!com.android.media.flags.Flags.enableSuggestedDeviceApi()) {

            return;

        }

        if (updateSuggestedDeviceState()) {

            dispatchOnSuggestedDeviceUpdated();

        }

        if (updateMediaDevicesSuggestionState()) {

            dispatchDeviceListAdded(mMediaDevices);

        }

    }



    private boolean updateSuggestedDeviceState() {

        if (!com.android.media.flags.Flags.enableSuggestedDeviceApi()) {

            return false;

        }

        SuggestedDeviceInfo topSuggestion = null;

        SuggestedDeviceState newSuggestedDeviceState = null;

        SuggestedDeviceState previousState = mSuggestedDeviceState;
@@ -720,6 +733,7 @@ public abstract class InfoMediaManager { 
            topSuggestion = suggestions.get(0);

        }

        if (topSuggestion != null) {

            synchronized (mLock) {

                for (MediaDevice device : mMediaDevices) {

                    if (Objects.equals(device.getId(), topSuggestion.getRouteId())) {

                        newSuggestedDeviceState =
@@ -727,23 +741,22 @@ public abstract class InfoMediaManager { 
                        break;

                    }

                }

            }

            if (newSuggestedDeviceState == null) {

                if (previousState != null

                        && topSuggestion

                                .getRouteId()

                                .equals(previousState.getSuggestedDeviceInfo().getRouteId())) {

                    return;

                    return false;

                }

                newSuggestedDeviceState = new SuggestedDeviceState(topSuggestion);

            }

        }

        if (updateMediaDevicesSuggestionState()) {

            dispatchDeviceListAdded(mMediaDevices);

        }

        if (!Objects.equals(previousState, newSuggestedDeviceState)) {

            mSuggestedDeviceState = newSuggestedDeviceState;

            dispatchOnSuggestedDeviceUpdated();

            return true;

        }

        return false;

    }



    final void onConnectionAttemptedForSuggestion(@NonNull SuggestedDeviceState suggestion) {
@@ -798,6 +811,9 @@ public abstract class InfoMediaManager { 


    // Go through all current MediaDevices, and update the ones that are suggested.

    private boolean updateMediaDevicesSuggestionState() {

        if (!com.android.media.flags.Flags.enableSuggestedDeviceApi()) {

            return false;

        }

        Set<String> suggestedDevices = new HashSet<>();

        // Prioritize suggestions from the package, otherwise pick any.

        List<SuggestedDeviceInfo> suggestions = getSuggestions();
@@ -807,6 +823,7 @@ public abstract class InfoMediaManager { 
            }

        }

        boolean didUpdate = false;

        synchronized (mLock) {

            for (MediaDevice device : mMediaDevices) {

                if (device.isSuggestedDevice()) {

                    if (!suggestedDevices.contains(device.getId())) {
@@ -816,26 +833,27 @@ public abstract class InfoMediaManager { 
                        if (!device.isSuggestedByRouteListingPreferences()) {

                            didUpdate = true;

                        }

                    // Case 2: Device was suggested by both setDeviceSuggestions() and RLP. Since

                    // it's still suggested by RLP, no update.

                        // Case 2: Device was suggested by both setDeviceSuggestions() and RLP.

                        // Since it's still suggested by RLP, no update.

                    } else {

                    // Case 3: Device was suggested (either by RLP or by setDeviceSuggestions()),

                    // and should still be suggested.

                        // Case 3: Device was suggested (either by RLP or by

                        // setDeviceSuggestions()), and should still be suggested.

                        device.setIsSuggested(true);

                    }

                } else {

                    if (suggestedDevices.contains(device.getId())) {

                    // Case 4: Device was not suggested by either RLP or setDeviceSuggestions() but

                    // is now suggested.

                        // Case 4: Device was not suggested by either RLP or setDeviceSuggestions()

                        // but is now suggested.

                        device.setIsSuggested(true);

                        didUpdate = true;

                    } else {

                    // Case 5: Device was not suggested by either RLP or setDeviceSuggestions() and

                    // is still not suggested.

                        // Case 5: Device was not suggested by either RLP or setDeviceSuggestions()

                        // and is still not suggested.

                        device.setIsSuggested(false);

                    }

                }

            }

        }



        return didUpdate;

    }
@@ -864,7 +882,6 @@ public abstract class InfoMediaManager { 
            // First device on the list is always the first selected route.

            mCurrentConnectedDevice = mMediaDevices.get(0);

        }

        updateDeviceSuggestion();

    }



    private synchronized List<MediaRoute2Info> getAvailableRoutes(
@@ -909,8 +926,8 @@ public abstract class InfoMediaManager { 
                getDynamicRouteAttributes(activeSession, route);

        MediaDevice mediaDevice = createMediaDeviceFromRoute(route, dynamicRouteAttributes);

        if (mediaDevice != null) {

            if (activeSession.getSelectedRoutes().contains(route.getId())) {

                setDeviceState(mediaDevice, STATE_SELECTED);

            if (mediaDevice.isSelected()) {

                mediaDevice.setState(STATE_SELECTED);

            }

            mMediaDevices.add(mediaDevice);

        }
@@ -973,7 +990,9 @@ public abstract class InfoMediaManager { 
        }

        device.setState(state);

        if (device.isSuggestedDevice()) {

            updateDeviceSuggestion();

            if (updateSuggestedDeviceState()) {

                dispatchOnSuggestedDeviceUpdated();

            }

        }

    }

packages/SettingsLib/src/com/android/settingslib/media/RouterInfoMediaManager.java

+3 −3
Original line number Diff line number Diff line
@@ -79,12 +79,12 @@ public final class RouterInfoMediaManager extends InfoMediaManager { 
                public void onSuggestionsUpdated(

                        String suggestingPackageName,

                        List<SuggestedDeviceInfo> suggestedDeviceInfo) {

                    updateDeviceSuggestion(suggestingPackageName, suggestedDeviceInfo);

                    notifyDeviceSuggestionUpdated(suggestingPackageName, suggestedDeviceInfo);

                }



                @Override

                public void onSuggestionsCleared(String suggestingPackageName) {

                    updateDeviceSuggestion(suggestingPackageName, null);

                    notifyDeviceSuggestionUpdated(suggestingPackageName, null);

                }



                @Override
@@ -165,7 +165,7 @@ public final class RouterInfoMediaManager extends InfoMediaManager { 
        if (Flags.enableSuggestedDeviceApi()) {

            for (Map.Entry<String, List<SuggestedDeviceInfo>> entry :

                    mRouter.getDeviceSuggestions().entrySet()) {

                updateDeviceSuggestion(entry.getKey(), entry.getValue());

                notifyDeviceSuggestionUpdated(entry.getKey(), entry.getValue());

            }

        }

        mRouter.registerTransferCallback(mExecutor, mTransferCallback);

packages/SettingsLib/tests/robotests/src/com/android/settingslib/media/InfoMediaManagerTest.java

+148 −0
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ import static android.media.MediaRoute2ProviderService.REASON_UNKNOWN_ERROR; 
import static com.android.settingslib.media.LocalMediaManager.MediaDeviceState.STATE_SELECTED;



import static com.google.common.truth.Truth.assertThat;

import static com.google.common.truth.Truth.assertWithMessage;



import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyString;
@@ -58,6 +59,9 @@ import android.os.Build; 
import android.platform.test.annotations.EnableFlags;

import android.platform.test.flag.junit.SetFlagsRule;



import androidx.annotation.NonNull;

import androidx.annotation.Nullable;



import com.android.media.flags.Flags;

import com.android.settingslib.bluetooth.CachedBluetoothDevice;

import com.android.settingslib.bluetooth.CachedBluetoothDeviceManager;
@@ -83,6 +87,11 @@ import org.robolectric.util.ReflectionHelpers; 
import java.util.ArrayList;

import java.util.List;

import java.util.Set;

import java.util.concurrent.CountDownLatch;

import java.util.concurrent.ExecutorService;

import java.util.concurrent.Executors;

import java.util.concurrent.TimeUnit;

import java.util.concurrent.atomic.AtomicBoolean;



@RunWith(RobolectricTestRunner.class)

public class InfoMediaManagerTest {
@@ -135,6 +144,8 @@ public class InfoMediaManagerTest { 
                    .addFeature(MediaRoute2Info.FEATURE_LIVE_AUDIO)

                    .build();



    private static final int ASYNC_TIMEOUT_SECONDS = 5;



    @Rule public final SetFlagsRule mSetFlagsRule = new SetFlagsRule();



    @Mock
@@ -860,6 +871,143 @@ public class InfoMediaManagerTest { 
        verify(mCallback, times(2)).onDeviceListAdded(any());

    }



    @Test

    public void onDeviceListAdded_callingGettersInCallback_shouldNotCauseDeadlock()

            throws InterruptedException {

        when(mRouterManager.getRoutingSessions(anyString())).thenReturn(

                List.of(TEST_SYSTEM_ROUTING_SESSION));

        when(mRouterManager.getSelectedRoutes(any())).thenReturn(

                List.of(TEST_SELECTED_SYSTEM_ROUTE));



        ExecutorService executor = Executors.newSingleThreadExecutor();



        final CountDownLatch callbackEnteredLatch = new CountDownLatch(1);

        final CountDownLatch mainThreadActionLatch = new CountDownLatch(1);



        final AtomicBoolean callbackCompletedSuccessfully = new AtomicBoolean(false);



        InfoMediaManager.MediaDeviceCallback mediaDeviceCallback =

                new InfoMediaManager.MediaDeviceCallback() {

                    @Override

                    public void onDeviceListAdded(@NonNull List<MediaDevice> devices) {

                        try {

                            callbackEnteredLatch.countDown();

                            // Pausing the callback and waiting for the main thread to act.

                            boolean mainThreadUnblocked = mainThreadActionLatch.await(

                                    ASYNC_TIMEOUT_SECONDS, TimeUnit.SECONDS);

                            callbackCompletedSuccessfully.set(mainThreadUnblocked);

                        } catch (InterruptedException e) {

                            Thread.currentThread().interrupt();

                        }

                    }



                    @Override

                    public void onDeviceListRemoved(@NonNull List<MediaDevice> devices) {

                    }



                    @Override

                    public void onConnectedDeviceChanged(@Nullable String id) {

                    }



                    @Override

                    public void onRequestFailed(int reason) {

                    }

                };





        // This will invoke a callback in the background thread.

        executor.submit(() -> mInfoMediaManager.registerCallback(mediaDeviceCallback));



        if (!callbackEnteredLatch.await(ASYNC_TIMEOUT_SECONDS, TimeUnit.SECONDS)) {

            throw new AssertionError("Callback was never entered.");

        }



        // While the callback execution is on pause, make a call to a potential blocking method.

        mInfoMediaManager.getSelectedMediaDevices();



        // Signal the waiting callback thread that it can now proceed.

        mainThreadActionLatch.countDown();

        executor.shutdown();

        assertWithMessage("Deadlock detected! The test timed out.").that(

                executor.awaitTermination(ASYNC_TIMEOUT_SECONDS, TimeUnit.SECONDS)).isTrue();



        assertThat(callbackCompletedSuccessfully.get()).isTrue();

    }



    @Test

    @EnableFlags(Flags.FLAG_ENABLE_SUGGESTED_DEVICE_API)

    public void onSuggestedDeviceUpdated_callingGettersInCallback_shouldNotCauseDeadlock()

            throws InterruptedException {

        ExecutorService executor = Executors.newSingleThreadExecutor();



        final CountDownLatch callbackEnteredLatch = new CountDownLatch(1);

        final CountDownLatch mainThreadActionLatch = new CountDownLatch(1);



        final AtomicBoolean callbackCompletedSuccessfully = new AtomicBoolean(false);



        InfoMediaManager.MediaDeviceCallback mediaDeviceCallback =

                new InfoMediaManager.MediaDeviceCallback() {

                    @Override

                    public void onDeviceListAdded(@NonNull List<MediaDevice> devices) {

                    }



                    @Override

                    public void onDeviceListRemoved(@NonNull List<MediaDevice> devices) {

                    }



                    @Override

                    public void onConnectedDeviceChanged(@Nullable String id) {

                    }



                    @Override

                    public void onRequestFailed(int reason) {

                    }



                    @Override

                    public void onSuggestedDeviceUpdated(

                            @Nullable SuggestedDeviceState suggestedDevice) {

                        try {

                            callbackEnteredLatch.countDown();

                            // Pausing the callback and waiting for the main thread to act.

                            boolean mainThreadUnblocked = mainThreadActionLatch.await(

                                    ASYNC_TIMEOUT_SECONDS, TimeUnit.SECONDS);

                            callbackCompletedSuccessfully.set(mainThreadUnblocked);

                        } catch (InterruptedException e) {

                            Thread.currentThread().interrupt();

                        }

                    }

                };



        SuggestedDeviceInfo suggestedDeviceInfo = new SuggestedDeviceInfo.Builder("device_name",

                TEST_ID_3, 0).build();

        setAvailableRoutesList(TEST_PACKAGE_NAME);



        // This will invoke a callback in the background thread.

        executor.submit(() -> {

            mInfoMediaManager.registerCallback(mediaDeviceCallback);

            verify(mRouter2).registerDeviceSuggestionsUpdatesCallback(any(),

                    mDeviceSuggestionsUpdatesCallback.capture());



            mDeviceSuggestionsUpdatesCallback.getValue().onSuggestionsUpdated("random_package_name",

                    List.of(suggestedDeviceInfo));

        });



        if (!callbackEnteredLatch.await(ASYNC_TIMEOUT_SECONDS, TimeUnit.SECONDS)) {

            throw new AssertionError("Callback was never entered.");

        }



        // While the callback execution is on pause, make a call to a potential blocking method.

        mInfoMediaManager.getSelectedMediaDevices();



        // Signal the waiting callback thread that it can now proceed.

        mainThreadActionLatch.countDown();

        executor.shutdown();

        assertWithMessage("Deadlock detected! The test timed out.").that(

                executor.awaitTermination(ASYNC_TIMEOUT_SECONDS, TimeUnit.SECONDS)).isTrue();



        assertThat(callbackCompletedSuccessfully.get()).isTrue();

    }



    @Test

    public void addMediaDevice_verifyDeviceTypeCanCorrespondToMediaDevice() {

        final MediaRoute2Info route2Info = mock(MediaRoute2Info.class);