Fixed
Status Update
Comments
er...@google.com <er...@google.com> #2
I haven't looks on repro yet, but there is no such thing as AlwaysOnLifecycle in latest versions, LiveData has strong references on all observers (including that were added as observeForever). If nothings keeps a livedata, then it is gced.
er...@google.com <er...@google.com> #3
ah I see: in observe with Lifecycle we add LiveData as observer to Lifecycle and it keeps it alive. However keeping observerforever + livedata, when nothing has reference on them, is a memory leak by definition: user can't clean this memory anyhow, because neither observer / livedata could be referenced.
ch...@google.com <ch...@google.com>
ch...@google.com <ch...@google.com> #4
It is true, though from usage perspective, it is not obvious at all.
e.g. you are in a class, like a singleton repository where you want to observe the database for some query.
and you write:
MyRepository(dao : MyDao) {
dao.someLiveData().observeForever(....do something...);
}
And it suddenly stops working :/ which is very non-obvious from the developer's perspective.
If we kept the lifecycle in memory as long as it has observers, wouldn't it solve the problem since they can break the chain by calling stopObserving(observer) ?
When they use observeForever, they are already responsible to remove the observer so I think it is fine?
e.g. you are in a class, like a singleton repository where you want to observe the database for some query.
and you write:
MyRepository(dao : MyDao) {
dao.someLiveData().observeForever(....do something...);
}
And it suddenly stops working :/ which is very non-obvious from the developer's perspective.
If we kept the lifecycle in memory as long as it has observers, wouldn't it solve the problem since they can break the chain by calling stopObserving(observer) ?
When they use observeForever, they are already responsible to remove the observer so I think it is fine?
ch...@google.com <ch...@google.com>
vi...@gmail.com <vi...@gmail.com> #5
well, the issue is: no reference on observer, you can't break the chain because you don't have a reference. If you have a reference, LiveData (unfortunately) wouldn't be gc-ed, because even AlwaysActiveObserver inrenally has strong reference on livedata.
ch...@google.com <ch...@google.com> #6
It turns out I had an app with the exact above use case: a singleton repository observing a database. The code was working properly with arch components 1.0.0 but unpredictably with 1.1.0. I think this is a good example of a leaky abstraction.
Since I want to observe the database forever with no intention to remove the observer at any point, I had no reason to keep a reference to the LiveData and it ended up being GCed along with the observers in it.
You could also argue that the problem is not with LiveData but with Room using weak references to observers but the developer is not supposed to know the Room implementation details.
Since I want to observe the database forever with no intention to remove the observer at any point, I had no reason to keep a reference to the LiveData and it ended up being GCed along with the observers in it.
You could also argue that the problem is not with LiveData but with Room using weak references to observers but the developer is not supposed to know the Room implementation details.
vi...@gmail.com <vi...@gmail.com> #7
I think the real problem is that LiveData has no callback to know that no one wants it anymore; like onReset in Loader.
So there's 2 options for database observers:
1. It either has to un-register observer in onInactive and on next onActive re-query db. Problematic is if nothing has changed you have to filter values or dispatch unnecessarily, and you'll pay the price of query which could be expensive.
2. Register weak observer, or have wear ref to LiveData
Room uses option 2. I created my own LiveData and I used the same (https://bitbucket.org/snippets/Boza-s6/xe8749/sample-livedata-with-features-like-loaders#EmailContentLiveData.java-104 ), because it was obvious solution, but now I see it will not work in every possible case.
I think there should be statically referenced Lifecycle object to which LiveData subscribes when using observeForever() (and observer is anonymous class, maybe?).
So there's 2 options for database observers:
1. It either has to un-register observer in onInactive and on next onActive re-query db. Problematic is if nothing has changed you have to filter values or dispatch unnecessarily, and you'll pay the price of query which could be expensive.
2. Register weak observer, or have wear ref to LiveData
Room uses option 2. I created my own LiveData and I used the same (
I think there should be statically referenced Lifecycle object to which LiveData subscribes when using observeForever() (and observer is anonymous class, maybe?).
ch...@google.com <ch...@google.com> #8
I ran into the same problem yesterday. It tooked me hours to understand what is happening and why my observeForever-observer suddenly stopped being invoked.
I think it is a really bad design that someone should know the implementation details of generated Room DB Code.
I want to use observerForever the same way like i use the simple observe and dont want to ran into problems because of internal implementation details.
I think it is a really bad design that someone should know the implementation details of generated Room DB Code.
I want to use observerForever the same way like i use the simple observe and dont want to ran into problems because of internal implementation details.
vi...@gmail.com <vi...@gmail.com> #9
Project: platform/frameworks/support
Branch: androidx-master-dev
commit bab31cacd4ae98057b694f6ae4c781f5f25ee0c9
Author: Yigit Boyar <yboyar@google.com>
Date: Wed Oct 31 23:41:00 2018
Fix live data observe forever in room
This CL fixes a problem where if DAO returned LiveData is
observed with observeForever but there are no references
to the LiveData, it would eventually be garbage collected.
To avoid this, InvalidationTracker now manages these LiveData
objects and strongly references them as long as there is an
active observer.
To be 100%, we should actually do it when LiveData has observers
but right now that API does not exist in the LiveData side
to get notified when the observer count changes 0 and 1.
I've also cleaned up some test code to avoid duplicating the
test observers in each test.
I've also added an integration test that uses room w/o any of
the optional dependencies to ensure that we don't break the
contract on compileOnly dependencies.
Bug: 74477406
Test: LiveDataQueryTest, InvalidationTrackerTest, NoAppCompatTestApp
Change-Id: If88114306d7e20ddaed0312a1203a0d14b710145
M arch/core-testing/src/main/java/androidx/arch/core/executor/TaskExecutorWithFakeMainThread.java
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/LiveDataQueryResultBinder.kt
M room/compiler/src/test/data/daoWriter/output/ComplexDao.java
A room/integration-tests/noappcompattestapp/.gitignore
A room/integration-tests/noappcompattestapp/README.md
A room/integration-tests/noappcompattestapp/build.gradle
A room/integration-tests/noappcompattestapp/src/androidTest/java/androidx/room/integration/noappcompat/BareDatabaseTest.java
A room/integration-tests/noappcompattestapp/src/main/AndroidManifest.xml
A room/integration-tests/noappcompattestapp/src/main/java/androidx/room/integration/noappcompat/BareDatabase.java
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/paging/DataSourceFactoryTest.java
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/InvalidationTrackerTest.java
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/LiveDataQueryTest.java
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/QueryTransactionTest.java
A room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/TestLifecycleOwner.java
A room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/TestObserver.java
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/TestUtil.java
M room/runtime/build.gradle
A room/runtime/src/main/java/androidx/room/InvalidationLiveDataContainer.java
M room/runtime/src/main/java/androidx/room/InvalidationTracker.java
A room/runtime/src/main/java/androidx/room/RoomTrackingLiveData.java
A room/runtime/src/test/java/androidx/room/InvalidationLiveDataContainerTest.kt
M room/runtime/src/test/java/androidx/room/InvalidationTrackerTest.java
M settings.gradle
https://android-review.googlesource.com/810053
https://goto.google.com/android-sha1/bab31cacd4ae98057b694f6ae4c781f5f25ee0c9
Branch: androidx-master-dev
commit bab31cacd4ae98057b694f6ae4c781f5f25ee0c9
Author: Yigit Boyar <yboyar@google.com>
Date: Wed Oct 31 23:41:00 2018
Fix live data observe forever in room
This CL fixes a problem where if DAO returned LiveData is
observed with observeForever but there are no references
to the LiveData, it would eventually be garbage collected.
To avoid this, InvalidationTracker now manages these LiveData
objects and strongly references them as long as there is an
active observer.
To be 100%, we should actually do it when LiveData has observers
but right now that API does not exist in the LiveData side
to get notified when the observer count changes 0 and 1.
I've also cleaned up some test code to avoid duplicating the
test observers in each test.
I've also added an integration test that uses room w/o any of
the optional dependencies to ensure that we don't break the
contract on compileOnly dependencies.
Bug: 74477406
Test: LiveDataQueryTest, InvalidationTrackerTest, NoAppCompatTestApp
Change-Id: If88114306d7e20ddaed0312a1203a0d14b710145
M arch/core-testing/src/main/java/androidx/arch/core/executor/TaskExecutorWithFakeMainThread.java
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/LiveDataQueryResultBinder.kt
M room/compiler/src/test/data/daoWriter/output/ComplexDao.java
A room/integration-tests/noappcompattestapp/.gitignore
A room/integration-tests/noappcompattestapp/README.md
A room/integration-tests/noappcompattestapp/build.gradle
A room/integration-tests/noappcompattestapp/src/androidTest/java/androidx/room/integration/noappcompat/BareDatabaseTest.java
A room/integration-tests/noappcompattestapp/src/main/AndroidManifest.xml
A room/integration-tests/noappcompattestapp/src/main/java/androidx/room/integration/noappcompat/BareDatabase.java
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/paging/DataSourceFactoryTest.java
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/InvalidationTrackerTest.java
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/LiveDataQueryTest.java
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/QueryTransactionTest.java
A room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/TestLifecycleOwner.java
A room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/TestObserver.java
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/TestUtil.java
M room/runtime/build.gradle
A room/runtime/src/main/java/androidx/room/InvalidationLiveDataContainer.java
M room/runtime/src/main/java/androidx/room/InvalidationTracker.java
A room/runtime/src/main/java/androidx/room/RoomTrackingLiveData.java
A room/runtime/src/test/java/androidx/room/InvalidationLiveDataContainerTest.kt
M room/runtime/src/test/java/androidx/room/InvalidationTrackerTest.java
M settings.gradle
vi...@gmail.com <vi...@gmail.com> #10
FYI this requires client to update to Room 2.1.0-alpha03 but it is fully compatible with 1.x that WM uses.
vi...@gmail.com <vi...@gmail.com> #11
actually it won't work because the change is in generated code. I'll try to backport itinto WM.
ch...@google.com <ch...@google.com> #12
re-opening for WM part.
vi...@gmail.com <vi...@gmail.com> #13
Project: platform/frameworks/support
Branch: androidx-master-dev
commit 3fb46bd8602faaf9c34aa969f8ae589c251c2f72
Author: Yigit Boyar <yboyar@google.com>
Date: Fri Nov 30 13:19:34 2018
Track LiveData returned by the WM
This CL works around a bug in Room 1.x where LiveData returned
by Room might be garbage collected.
Now work manager tracks any live data it returns as long as the
LiveData is active.
Bug: 74477406
Test: ObserForeverTest, WorkManagerLiveDataTrackerTest
Change-Id: I1e45dade1e8a3c29d6c35f7cb0c6e072c68ee658
A work/workmanager/src/androidTest/java/androidx/work/impl/ObserveForeverTest.java
M work/workmanager/src/main/java/androidx/work/impl/WorkManagerImpl.java
A work/workmanager/src/main/java/androidx/work/impl/WorkManagerLiveDataTracker.java
A work/workmanager/src/test/java/androidx/work/impl/WorkManagerLiveDataTrackerTest.java
https://android-review.googlesource.com/838833
https://goto.google.com/android-sha1/3fb46bd8602faaf9c34aa969f8ae589c251c2f72
Branch: androidx-master-dev
commit 3fb46bd8602faaf9c34aa969f8ae589c251c2f72
Author: Yigit Boyar <yboyar@google.com>
Date: Fri Nov 30 13:19:34 2018
Track LiveData returned by the WM
This CL works around a bug in Room 1.x where LiveData returned
by Room might be garbage collected.
Now work manager tracks any live data it returns as long as the
LiveData is active.
Bug: 74477406
Test: ObserForeverTest, WorkManagerLiveDataTrackerTest
Change-Id: I1e45dade1e8a3c29d6c35f7cb0c6e072c68ee658
A work/workmanager/src/androidTest/java/androidx/work/impl/ObserveForeverTest.java
M work/workmanager/src/main/java/androidx/work/impl/WorkManagerImpl.java
A work/workmanager/src/main/java/androidx/work/impl/WorkManagerLiveDataTracker.java
A work/workmanager/src/test/java/androidx/work/impl/WorkManagerLiveDataTrackerTest.java
ch...@google.com <ch...@google.com> #14
I missed some settings when I simulated the issue by robolectric test so that I was not able to reproduce it. Now, I can reproduce the issue if the device only supports one 480x480 resolution. I'm working on the solution and target to make it included in next release.
ap...@google.com <ap...@google.com> #15
Project: platform/frameworks/support
Branch: androidx-main
commit 69d15dff7bb857ee33a0f643ff42a0f8bc475ab2
Author: charcoalchen <charcoalchen@google.com>
Date: Fri Jan 08 18:30:03 2021
Fixed IllegalArgumentException issue happened when all preview supported sizes are smaller than 640x480 and display size is larger than 640x480.
Do not filter out sizes smaller than 640x480 when all preview supported sizes are smaller than 640x480 and display size is larger than 640x480.
Relnote:"Fixed IllegalArgumentException issue happened when all preview supported sizes are smaller than 640x480 and display size is larger than 640x480."
Bug: 150506192
Test: SupportedSurfaceCombinationTest
Change-Id: I2a63ce8e2ad42a9cc060c8635ac3603bf440b1ec
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/SupportedSurfaceCombination.java
M camera/camera-camera2/src/test/java/androidx/camera/camera2/internal/SupportedSurfaceCombinationTest.java
https://android-review.googlesource.com/1545624
Branch: androidx-main
commit 69d15dff7bb857ee33a0f643ff42a0f8bc475ab2
Author: charcoalchen <charcoalchen@google.com>
Date: Fri Jan 08 18:30:03 2021
Fixed IllegalArgumentException issue happened when all preview supported sizes are smaller than 640x480 and display size is larger than 640x480.
Do not filter out sizes smaller than 640x480 when all preview supported sizes are smaller than 640x480 and display size is larger than 640x480.
Relnote:"Fixed IllegalArgumentException issue happened when all preview supported sizes are smaller than 640x480 and display size is larger than 640x480."
Bug: 150506192
Test: SupportedSurfaceCombinationTest
Change-Id: I2a63ce8e2ad42a9cc060c8635ac3603bf440b1ec
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/SupportedSurfaceCombination.java
M camera/camera-camera2/src/test/java/androidx/camera/camera2/internal/SupportedSurfaceCombinationTest.java
Description
1.0.0-alpha07
DEVICE NAME: (Nexus 5X, Samsung S6, etc)
name : FUJITSU arrowsM04
Android OS version : 7.1.1
name: FUJITSU F-04J
Android OS version : 6.0.1
name: FUJITSU F-05J
Android OS version : 7.1.1
DESCRIPTION:
We receive following crash report from our app user.
Fatal Exception: java.lang.IllegalArgumentException: Can not get supported output size under supported maximum for the format: 34
at a.c.a.b.k0.a + 98(SupportedSurfaceCombination.java:98)
at a.c.a.b.k0.a + 30(SupportedSurfaceCombination.java:30)
at a.c.a.b.t.a + 22(Camera2DeviceSurfaceManager.java:22)
at androidx.camera.core.k1.a + 98(CameraX.java:98)
at androidx.camera.core.k1.a + 22(CameraX.java:22)
at a.c.b.c.a + 6(ProcessCameraProvider.java:6)
at <out app package name>.CameraFragment.a + 33(CameraFragment.kt:33)
at <out app package name>.CameraFragment.a + 1(CameraFragment.kt:1)
at <out app package name>.CameraFragment$m.run + 1(CameraFragment.kt:1)
at android.os.Handler.handleCallback + 751(Handler.java:751)
at android.os.Handler.dispatchMessage + 95(Handler.java:95)
at android.os.Looper.loop + 154(Looper.java:154)
at android.app.ActivityThread.main + 6262(ActivityThread.java:6262)
at java.lang.reflect.Method.invoke(Method.java)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run + 890(ZygoteInit.java:890)
at com.android.internal.os.ZygoteInit.main + 780(ZygoteInit.java:780)
REPRODUCIBILITY: (5 of out 5, 1 out of 100, etc)
We receive this crash report from only three devices. And these devices's manufacturer is FUJITSU.
ADDITIONAL INFORMATION:
CODE FRAGMENTS (this will help us troubleshoot your issues):
Our app bind usecases by following code.
private fun bindCameraUseCases(processCameraProvider: ProcessCameraProvider) {
preview = Preview.Builder().apply {
setTargetAspectRatio(AspectRatio.RATIO_4_3)
}.build().apply {
previewSurfaceProvider = binding.previewViewCamera.previewSurfaceProvider
}
imageCapture = ImageCapture.Builder().apply {
setCaptureMode(ImageCapture.CaptureMode.MINIMIZE_LATENCY)
setTargetAspectRatio(AspectRatio.RATIO_4_3)
setFlashMode(FlashMode.OFF)
}.build()
val cameraSelector = CameraSelector.Builder().requireLensFacing(LensFacing.BACK).build()
processCameraProvider.bindToLifecycle(viewLifecycleOwner, cameraSelector, preview, imageCapture)
}