I hope to add method in UnsafeFragmentLifecycleObserverDetector [148996309]

Fixed

Bug

Status Update

No update yet.

Description

uc...@gmail.com

created issue #1

Feb 6, 2020 12:01PM

Component used: Fragment
Version used:
androidx.fragment:fragment:1.2.0
androidx.lifecycle:lifecycle-lifedata:2.2.0

Devices/Android versions reproduced on: Android studio lint issue. (Android Studio 4.0 canary 9)

I like your lint detector and I agree UnsafeFragmentLifecycleObserverDetector's direction.

UNSAFE_METHODS is like that.
internal val UNSAFE_METHODS = mapOf(
Method("androidx.lifecycle.LiveData", "observe") to LIVEDATA_ISSUE,
Method("androidx.activity.OnBackPressedDispatcher", "addCallback") to BACK_PRESSED_ISSUE
)
(SourceCode referenced is :

https://android.googlesource.com/platform/frameworks/support/+/androidx-master-dev/fragment/fragment-lint/src/main/java/androidx/fragment/lint/UnsafeFragmentLifecycleObserverDetector.kt )

And 'inline fun <T> LiveData<T>.observe' was add in 'androidx.lifecycle:lifecycle-livedata-core-ktx:2.2.0'.
It isn't catch by lint in "onCreateView", "onViewCreated", "onActivityCreated", "onViewStateRestored". When I used viewModel.liveData.observe(this) { } in Fragment.

So I hope to add one unsafe method in map of 'UNSAFE_METHODS'.
Method("androidx.lifecycle.LiveDataKt", "observe") to LIVEDATA_ISSUE,

I was test in my example and works good.

Let me know your opinion.

Best Regards.

If this is a bug in the library, we would appreciate if you could attach:
- Sample project to trigger the issue.
- A screenrecord or screenshots showing the issue (if UI related).

Comments

il...@google.com <il...@google.com> #2Feb 6, 2020 05:28PM

Yigit, do you have time to fix it?

reemission of the same liveData is racy

uc...@gmail.com <uc...@gmail.com> #3Feb 6, 2020 07:55PM

yea i'll take it.

il...@google.com <il...@google.com> Feb 7, 2020 10:03PM

Assigned to jb...@google.com.

jb...@google.com <jb...@google.com> #4Feb 10, 2020 11:01PM

Thanks for the detailed analysis. This may not be an issue anymore since we've started using Main.immediate there but I' not sure; I'll try to create a test case.

ap...@google.com <ap...@google.com> #5Feb 11, 2020 12:00AM

just emitting same live data reproduces the issue.

@Test
fun raceTest() {
val subLiveData = MutableLiveData(1)
val subject = liveData(testScope.coroutineContext) {
emitSource(subLiveData)
emitSource(subLiveData) //crashes
}
subject.addObserver().apply {
testScope.advanceUntilIdle()
}
}

an...@google.com <an...@google.com> #6Feb 11, 2020 06:19AM

With 2.2.0-alpha04 (that use Main.immediate), the issue seems to be still there (I tested it by calling emitSource() twice, like your test case)

jb...@google.com <jb...@google.com> #7Feb 12, 2020 12:42AM

Marked as fixed.

yea sorry immediate does not fix it.
I actually have a WIP fix for it:

https://android-review.googlesource.com/c/platform/frameworks/support/+/1112186

if your case is the one i found (emitting same LiveData multiple times, as shown in #5) you can work around it by adding a dummy transformation.

val subLiveData = MutableLiveData(1)
val subject = liveData(testScope.coroutineContext) {
emitSource(subLiveData.map {it })
emitSource(subLiveData.map {it} )
}