Merge "Prevent invoking callbacks from `synchronized` block." into main (e95dcb56) · Commits · e / os / android_frameworks_base

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