observe forever get GC'ed [74477406]

Fixed

Bug

Status Update

No update yet.

Description

yb...@google.com

created issue #1

Mar 10, 2018 08:46PM

repro app:

https://github.com/cbeyls/RoomObserveForeverBug

repro steps from the developer:
launch the app on a device, press the button to update the counters. Then after you use "force gc" from android monitor, the second counter will stop updating

Looks like because nothing keeps a reference to the AlwaysOnLifecycle or the LiveData, both end up being GC'ed. Before it was a single instance so it didn't get GC'ed.
We probably need to keep AlwaysOnLifecycle in memory as long as it has an observer.

Comments

se...@google.com <se...@google.com> #2Mar 11, 2018 01:45AM

Rahul, can we reasonably remove this in 1.3?

I think the slices like "Choreographer#doframe - resynced to" were one source of this, which is now filtered out explicitly here

se...@google.com <se...@google.com> #3Mar 11, 2018 01:59AM

Moving this to Benchmark 1.4 because we need to investigate if there are other reasons for us to be using the sub-expression slice.parent_stack_id = 0.

yb...@google.com <yb...@google.com> #4Mar 11, 2018 02:05AM

I also have this issue on Android 14. i see that there's another slice that has Choreographer#doFrame as its child: "`android.view.Choreographer$FrameDisplayEventReceiver@b5d5d6e Handler (android.view.Choreographer$FrameHandler) `"

I attached screenshot from perfetto

se...@google.com <se...@google.com> #5Mar 11, 2018 02:14AM

Should be addressed by aosp/3498998

ch...@beyls.net <ch...@beyls.net> #6Mar 12, 2018 12:35PM

Project: platform/frameworks/support
Branch: androidx-main
Author: Rahul Ravikumar <rahulrav@google.com>
Link: https://android-review.googlesource.com/3498998

No longer assume Choreographer#doFrame is the top of the stack frame on the main thread for FrameTimingQuery.

Expand for full commit details

No longer assume `Choreographer#doFrame` is the top of the stack frame on the main thread for FrameTimingQuery. 
 
* This is no longer necessary given we explicitly filter out `resynced`-ed frames. 
 
Test: Existing FrameTimingQueryTests pass. 
Fixes: 340206285 
Relnote: No longer assume `Choreographer#doFrame` is the top of the stack frame on the main thread for FrameTimingQuery. 
Change-Id: Iee0e0bec02e2af6f675b48384d14a59569b3dc57

Files:

M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/perfetto/FrameTimingQuery.kt

Hash: ae475a6c5e1fcaade64047f487e425643fcef368
Date: Tue Feb 18 15:33:26 2025

ne...@gmail.com <ne...@gmail.com> #7May 27, 2018 12:18PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.benchmark:benchmark-macro:1.4.0-alpha09

m....@gmail.com <m....@gmail.com> #8Jun 1, 2018 06:10AM

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.

yb...@google.com <yb...@google.com> Oct 31, 2018 11:41PM

Reassigned to yb...@google.com.

yb...@google.com <yb...@google.com> Nov 28, 2018 09:32PM

Accepted by yb...@google.com.

ap...@google.com <ap...@google.com> #9Nov 29, 2018 04:06PM

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

yb...@google.com <yb...@google.com> Nov 29, 2018 04:06PM

Marked as fixed.

yb...@google.com <yb...@google.com> #10Nov 30, 2018 06:07PM

FYI this requires client to update to Room 2.1.0-alpha03 but it is fully compatible with 1.x that WM uses.

yb...@google.com <yb...@google.com> #11Nov 30, 2018 06:24PM

actually it won't work because the change is in generated code. I'll try to backport itinto WM.

yb...@google.com <yb...@google.com> #12Nov 30, 2018 06:27PM

Status: Assigned (reopened)

re-opening for WM part.

ap...@google.com <ap...@google.com> #13Dec 3, 2018 07:23PM

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

yb...@google.com <yb...@google.com> Dec 3, 2018 09:22PM