Fixed
Status Update
Comments
ap...@google.com <ap...@google.com> #2
Project: platform/frameworks/support
Branch: androidx-main
commit 8e88e3cf7f8c40b3fda166f0beb5a577ba15d32b
Author: Daniel Santiago Rivera <danysantiago@google.com>
Date: Wed Jul 03 11:04:33 2024
Migrate MultiInstanceInvalidationClient to Coroutines
This change moves the invalidation client to Coroutines, aligning with the rest of the Coroutine-based Invalidation Tracker refactor for KMP. It also changes when the 'all-tables' observer is registered. The observer will now be registered when the client is created in an start(), which should always be on a background thread during InvalidationTracker.internalInit(). This reduces the likelihood of missing invalidations ( b/324260327 ) it also keeps things simple and avoids the racing nature of onServiceConnected and onServiceDisconnected being used to register / unregister the observer ( b/350927013 ).
This CL also greatly cleans up the MultiInstanceInvalidationTest which has been plague with flaky and inconsistent tests due to its concurrency nature. First, it removes the benchmark relates tests, they need to be properly moved to the benchmark integration project. Second it removes asserting behaviour using LiveData, since there is a lot of thread hopping it is harder to keep it deterministic yet it doesn't provide additional coverage, as long as observers are asserted to be invalidated it is expected then that observers that back LiveData would also be invalidated.
Bug: 309996304
Bug: 350927013
Bug: 324260327
Bug: 324274513
Bug: 335890993
Bug: 330519843
Bug: 186405912
Test: MultiInstanceInvalidationTest
Change-Id: I26cbc06c752da7e465dc9ec8831ef5bc5e343606
M room/integration-tests/testapp/lint-baseline.xml
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/MultiInstanceInvalidationTest.java
M room/integration-tests/testapp/src/main/java/androidx/room/integration/testapp/SampleDatabaseService.java
M room/integration-tests/testapp/src/main/java/androidx/room/integration/testapp/database/CustomerDao.java
M room/room-runtime/src/androidInstrumentedTest/kotlin/androidx/room/MultiInstanceInvalidationTest.kt
M room/room-runtime/src/androidMain/kotlin/androidx/room/InvalidationTracker.android.kt
M room/room-runtime/src/androidMain/kotlin/androidx/room/MultiInstanceInvalidationClient.android.kt
M room/room-runtime/src/commonMain/kotlin/androidx/room/InvalidationTracker.kt
https://android-review.googlesource.com/3159516
Branch: androidx-main
commit 8e88e3cf7f8c40b3fda166f0beb5a577ba15d32b
Author: Daniel Santiago Rivera <danysantiago@google.com>
Date: Wed Jul 03 11:04:33 2024
Migrate MultiInstanceInvalidationClient to Coroutines
This change moves the invalidation client to Coroutines, aligning with the rest of the Coroutine-based Invalidation Tracker refactor for KMP. It also changes when the 'all-tables' observer is registered. The observer will now be registered when the client is created in an start(), which should always be on a background thread during InvalidationTracker.internalInit(). This reduces the likelihood of missing invalidations (
This CL also greatly cleans up the MultiInstanceInvalidationTest which has been plague with flaky and inconsistent tests due to its concurrency nature. First, it removes the benchmark relates tests, they need to be properly moved to the benchmark integration project. Second it removes asserting behaviour using LiveData, since there is a lot of thread hopping it is harder to keep it deterministic yet it doesn't provide additional coverage, as long as observers are asserted to be invalidated it is expected then that observers that back LiveData would also be invalidated.
Bug: 309996304
Bug: 350927013
Bug: 324260327
Bug: 324274513
Bug: 335890993
Bug: 330519843
Bug: 186405912
Test: MultiInstanceInvalidationTest
Change-Id: I26cbc06c752da7e465dc9ec8831ef5bc5e343606
M room/integration-tests/testapp/lint-baseline.xml
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/MultiInstanceInvalidationTest.java
M room/integration-tests/testapp/src/main/java/androidx/room/integration/testapp/SampleDatabaseService.java
M room/integration-tests/testapp/src/main/java/androidx/room/integration/testapp/database/CustomerDao.java
M room/room-runtime/src/androidInstrumentedTest/kotlin/androidx/room/MultiInstanceInvalidationTest.kt
M room/room-runtime/src/androidMain/kotlin/androidx/room/InvalidationTracker.android.kt
M room/room-runtime/src/androidMain/kotlin/androidx/room/MultiInstanceInvalidationClient.android.kt
M room/room-runtime/src/commonMain/kotlin/androidx/room/InvalidationTracker.kt
da...@google.com <da...@google.com>
pr...@google.com <pr...@google.com> #3
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.room:room-runtime:2.7.0-alpha06
androidx.room:room-runtime-android:2.7.0-alpha06
androidx.room:room-runtime-iosarm64:2.7.0-alpha06
androidx.room:room-runtime-iossimulatorarm64:2.7.0-alpha06
androidx.room:room-runtime-iosx64:2.7.0-alpha06
androidx.room:room-runtime-jvm:2.7.0-alpha06
androidx.room:room-runtime-linuxx64:2.7.0-alpha06
androidx.room:room-runtime-macosarm64:2.7.0-alpha06
androidx.room:room-runtime-macosx64:2.7.0-alpha06
Description
To be specific, registering and unregistering observers is async and the client moves from main-thread `onServiceConnected` callback to background thread, but there might be some time before the runnable actually executes and it might be even possible that `onServiceDisconnected` get called, leading to scheduling another runnable that might or might not end up executing first.
I think the best solution is to finish the migration of the client to Coroutines.