Status Update
Comments
an...@gmail.com <an...@gmail.com> #2
ra...@google.com <ra...@google.com>
[Deleted User] <[Deleted User]> #3
gh...@google.com <gh...@google.com> #4
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?
an...@gmail.com <an...@gmail.com> #5
tn...@google.com <tn...@google.com> #6
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.
an...@gmail.com <an...@gmail.com> #7
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?).
an...@gmail.com <an...@gmail.com> #8
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.
wo...@gmail.com <wo...@gmail.com> #9
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
wo...@gmail.com <wo...@gmail.com> #10
di...@gmail.com <di...@gmail.com> #11
wo...@gmail.com <wo...@gmail.com> #12
[Deleted User] <[Deleted User]> #13
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
[Deleted User] <[Deleted User]> #14
We have checkDependencies=true
on our app module and apply there custom lint checks. Disabling custom lint checks has not solved the issue, though we have more custom lint checks via external dependencies (for example, timber).
an...@gmail.com <an...@gmail.com> #15
#14, could you check with checkDependencies=false
? It fixes problem on my project. Would be nice to hear more proves.
[Deleted User] <[Deleted User]> #16
#15, I've set checkDependencies = false
, but left our custom lint checks enabled. So far lint finished successfully without issue errors.
an...@gmail.com <an...@gmail.com> #17
In my case, all builds of the >50 were a success after checkDependencies = false
. However, disabling checkDependencies
is not a solution as well. Since all our code is split in small logical modules, there is no point in lint. Too many false-positives and true-negatives.
gh...@google.com <gh...@google.com> #18
I cherry-picked a change to 4.1 Beta 4 which essentially changes this error into a logged warning rather than a hard failure. That does not "fix" the problem, but it at least gets us back to the behavior of 3.6. Can someone confirm that this workaround has worked for them?
Also, I think I know what the root cause is (see notes below). However, it's hard to be sure without a repro scenario. Would anyone be willing to share an open-source project that reproduces this bug? (You can also send the project to
Notes:
-
My current theory is that this bug is related to
.https://issuetracker.google.com/159733104#comment6
I.e., I think Lint should not be reusing the same Kotlin compiler environment to analyze multiple modules, which is what we do whencheckDependencies=true
. -
It is still strange that the failure is nondeterministic. However, one potential reason is that the Kotlin compiler caches many things via
SoftReference
.
an...@gmail.com <an...@gmail.com> #19
Sorry for my long delay. I was tried with 4.1.0-beta05
(newest version to date), and the problem is still here. I added new log in case lines were changed.
Would anyone be willing to share an open-source project that reproduces this bug?
I have only closed source project with this problem. Maybe someone knows some quite large multimodule opensource project I could try setup AGP with lint and try reproduction?
an...@gmail.com <an...@gmail.com> #20
I tried to reproduce with two small multimodule projects, but all builds works flawlessly. Also, I tried to reproduce using checkDependencies
, and 6 other helper modules. 145k CLOC without tests. I suppose there is a good chance problem happen due to the cache.
It is still strange that the failure is nondeterministic.
It is even non-deterministic in CI/docker builds using the same fresh environment and exactly the same code each time. Build cache and daemon is disabled there. Multithreading should be involved as well.
In addition, I found in most cases failed due to the same file
br...@google.com <br...@google.com> #21
gh...@google.com <gh...@google.com> #22
There are some architectural changes coming in Lint that will fix this properly, but they will not make it into 4.2.
However, there is another workaround you can try that will be available starting in AGP 4.2 Beta 4.
Essentially you can tell Lint to "reset" the Kotlin compiler between each module analysis, thereby clearing all caches. I did not make this the default behavior because the effect on performance is unknown/risky.
To enable this behavior, either set the environment variable LINT_DO_NOT_REUSE_UAST_ENV
to "true", or set the JVM system property lint.do.not.reuse.uast.env
to "true". For example, you can do this from the command line:
./gradlew -Dlint.do.not.reuse.uast.env=true :app:lintDebug
Or by putting this in your gradle.properties
file:
systemProp.lint.do.not.reuse.uast.env=true
gh...@google.com <gh...@google.com> #23
Status updates:
-
I'm still hoping someone can confirm whether the workaround in
worked for them in AGP 4.2.comment#22 -
Starting with AGP 7.0-alpha13 the root cause of this issue should be fixed completely due to the new "partial analysis" mode (enabled by default) in which Lint analyzes modules independently and merges results along the way. It fixes the issue because Lint will no longer use the same Kotlin compiler environment to analyze multiple modules (and so the risk of stale caches is gone).
ws...@gmail.com <ws...@gmail.com> #24
I'm still hoping someone can confirm whether the workaround in
worked for them in AGP 4.2. comment#22
This unfortunately did not work for us. We actually see lint crashes after enabling via systemProp.lint.do.not.reuse.uast.env=true
. See the attached log. It's also much slower, as expected.
AGP 4.2 seems to make this error much more frequent compared to 4.1, for some reason.
gh...@google.com <gh...@google.com> #25
Can you file a new bug? You can add a comment here linking to it.
ws...@gmail.com <ws...@gmail.com> #26
Sure. Without that workaround, we get the same
ERROR: package fragment is not found for module:<lint-module> is a module[ModuleDescriptorImpl@5e5e6c61] file:KtFile: LiveViewerViewData.kt
error in this ticket occasionally (1-2% of the time?).
gh...@google.com <gh...@google.com> #27
Oh, I misunderstood. So, using lint.do.not.reuse.uast.env
is what causes the new crash? That is unfortunate, and I will investigate. Let me know if there is a sample project you can share that reproduces the issue.
ws...@gmail.com <ws...@gmail.com> #28
Not sure if you still wanted the new bug, but here it is:
gh...@google.com <gh...@google.com> #29
FYI the issue in
And to confirm, lint.do.not.reuse.uast.env
should no longer be needed at all in AGP 7.0+.
ku...@gmail.com <ku...@gmail.com> #30
Thanks #29 . I am noticing that when this happens, after the error log for
package fragment is not found for module:<lintWithKotlin> is a module
There is another error
java.lang.IllegalArgumentException: Argument for @NotNull parameter 'descriptor' of org/jetbrains/kotlin/codegen/context/CodegenContext.intoPackagePart must not be null
at org.jetbrains.kotlin.codegen.context.CodegenContext.$$$reportNull$$$0(CodegenContext.java)
at org.jetbrains.kotlin.codegen.context.CodegenContext.intoPackagePart(CodegenContext.java)
at org.jetbrains.kotlin.asJava.builder.LightClassDataProviderForClassOrObject$computeLightClassData$1$1.invoke(LightClassDataProvider.kt:51)
at org.jetbrains.kotlin.asJava.builder.LightClassDataProviderForClassOrObject$computeLightClassData$1$1.invoke(LightClassDataProvider.kt:38)
at org.jetbrains.kotlin.asJava.builder.LightClassBuilderKt.buildLightClass(LightClassBuilder.kt:64)
at org.jetbrains.kotlin.asJava.builder.LightClassDataProviderForClassOrObject$computeLightClassData$1.invoke(LightClassDataProvider.kt:47)
at org.jetbrains.kotlin.asJava.builder.LightClassDataProviderForClassOrObject$computeLightClassData$1.invoke(LightClassDataProvider.kt:38)
at org.jetbrains.kotlin.cli.jvm.compiler.CliLightClassGenerationSupport.createDataHolderForClass(CliLightClassGenerationSupport.kt:64)
at org.jetbrains.kotlin.asJava.builder.LightClassDataProviderForClassOrObject.computeLightClassData(LightClassDataProvider.kt:45)
at org.jetbrains.kotlin.asJava.builder.LightClassDataProviderForClassOrObject.compute(LightClassDataProvider.kt:61)
at com.intellij.psi.impl.PsiCachedValueImpl.doCompute(PsiCachedValueImpl.java:54)
at com.intellij.util.CachedValueBase.lambda$getValueWithLock$1(CachedValueBase.java:235)
at com.intellij.openapi.util.RecursionManager$1.doPreventingRecursion(RecursionManager.java:113)
at com.intellij.openapi.util.RecursionManager.doPreventingRecursion(RecursionManager.java:72)
at com.intellij.util.CachedValueBase.getValueWithLock(CachedValueBase.java:236)
at com.intellij.psi.impl.PsiCachedValueImpl.getValue(PsiCachedValueImpl.java:43)
at
Does the fix address both of these?
gh...@google.com <gh...@google.com> #31
package fragment is not found
and Argument for @NotNull parameter...
), the workaround in 4.2 is to put
systemProp.lint.do.not.reuse.uast.env=true
in your gradle.properties
file as described in
gh...@google.com <gh...@google.com> #32
Marking fixed in AGP 7.0.
That is, the lint.do.not.reuse.uast.env
workaround should no longer be needed in AGP 7.0+. And, you should no longer see the following warnings/errors while running Lint:
ERROR: package fragment is not found for module:<lintWithKotlin>...
ERROR: Could not generate LightClass for...
java.lang.IllegalArgumentException: Argument for @NotNull parameter 'descriptor' of org/jetbrains/kotlin/codegen/context/CodegenContext.intoPackagePart must not be null
The issue is fixed due to the new "Lint partial analysis" mode (enabled by default) which creates a standalone UastEnvironment
instance for each module, thereby avoiding stale caches in the Kotlin compiler frontend.
Please comment on the bug if you see any similar errors in AGP 7.0+.
Description
AGP: 4.0.0 Kotlin: 1.3.72 JVM: from docker image: 8u252-jdk-slim
Failure happens in CI builds, which always are clean. Failures are random. Sometimes builds are a success, but more often it is not. Each time random classes failed. For example, in this case, failed this little data class:
I included full stacktrace as file