Do not capture argument from ServiceListing.Callback

As the argument passed is the internal arraylist (mutable), capturing it
in the Runnable passed to the executor could cause a
ConcurrentModificationException.

Instead, map it before defining the Runnable so by the time the callback
returns, we don't have any copies of it.

The test checks that the original argument passed is the one that's
mapped and not a modified copy.

Test: atest ControlsListingControllerImpl
Fixes: 259222196
Change-Id: I6fff508dca50f0f9ac6981ad94e7cc9212cafda9
This commit is contained in:
Fabian Kozynski
2022-11-15 13:14:05 -05:00
parent b4c0589530
commit a6ab5b715c
2 changed files with 18 additions and 3 deletions

View File

@@ -91,11 +91,12 @@ class ControlsListingControllerImpl @VisibleForTesting constructor(
override var currentUserId = userTracker.userId
private set
private val serviceListingCallback = ServiceListing.Callback {
private val serviceListingCallback = ServiceListing.Callback { list ->
Log.d(TAG, "ServiceConfig reloaded, count: ${list.size}")
val newServices = list.map { ControlsServiceInfo(userTracker.userContext, it) }
// After here, `list` is not captured, so we don't risk modifying it outside of the callback
backgroundExecutor.execute {
if (userChangeInProgress.get() > 0) return@execute
Log.d(TAG, "ServiceConfig reloaded, count: ${it.size}")
val newServices = it.map { ControlsServiceInfo(userTracker.userContext, it) }
if (featureFlags.isEnabled(Flags.USE_APP_PANELS)) {
newServices.forEach(ControlsServiceInfo::resolvePanelActivity)
}

View File

@@ -46,6 +46,7 @@ import com.android.systemui.util.time.FakeSystemClock
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
@@ -480,6 +481,19 @@ class ControlsListingControllerImplTest : SysuiTestCase() {
assertNull(controller.getCurrentServices()[0].panelActivity)
}
@Test
fun testListingsNotModifiedByCallback() {
// This test checks that if the list passed to the callback is modified, it has no effect
// in the resulting services
val list = mutableListOf<ServiceInfo>()
serviceListingCallbackCaptor.value.onServicesReloaded(list)
list.add(ServiceInfo(ComponentName("a", "b")))
executor.runAllReady()
assertTrue(controller.getCurrentServices().isEmpty())
}
private fun ServiceInfo(
componentName: ComponentName,
panelActivityComponentName: ComponentName? = null