ResultWire issue

[MonStar1]

Looks like WeakReference lost the link to the listener. It happens randomly. I caught it in sample project at least three times (watch attached video with last one case). I know it is hard to understand from GIF what happen, better try to make the GIF slower I've added logs and you can see the listener is null inside WeakReference. It happens very often in my production project

[amatkivskiy]

Faced the same issue. 😞 Here are my logs:

2020-11-25 15:17:03.058 I:  adding key 'terms_accepted', current listeners: {}
2020-11-25 15:17:03.097 D: ViewModel@c1cc658.loadContent()
2020-11-25 15:17:03.811 D: ViewModel@c1cc658.onAcceptButtonClick()
2020-11-25 15:17:03.811 I: sendResult key 'terms_accepted', data: true
2020-11-25 15:17:03.811 I: printing listeners
2020-11-25 15:17:03.811 I: listener: terms_accepted=null

[terrakok]

Redundant listeners are cleared after sendResult. It is intentional behavior look here: https://github.com/terrakok/Cicerone/blob/master/library/src/main/kotlin/com/github/terrakok/cicerone/ResultWire.kt#L20

[amatkivskiy]

@terrakok Yeah, sorry for the confusion. Here is the code for logging:

internal class ResultWire {

    fun sendResult(key: String, data: Any) {
        println("sendResult key '$key', data: $data")
        printListeners()
        listeners.remove(key)?.get()?.let { listener ->
            println("found listener '$listener'")
            listener.onResult(data)
        }
    }

    fun printListeners() {
        println("printing listeners")
        listeners.onEach {
            println("listener: ${it.key}=${it.value.get()}")
        }
    }

    fun flush() {
        listeners.entries
            .filter { it.value.get() == null }
            .forEach {
                println("removing '${it.key}'")
                listeners.remove(it.key)
            }
    }
}

[amatkivskiy]

@terrakok here is code that triggers screen in viewmodel:

router.setResultListener("terms_accepted") { data ->
    areTermsAndConditionsAccepted = data as Boolean
}
router.navigateTo(Screens.termsAndConditions)

here is termsAndConditions screen viewmodel:

fun onAcceptButtonClick() {
    router.sendResult("terms_accepted", true)
    router.exit()
}

[terrakok]

Oh! I've understood! ResultWire saves weak ref to lambda so it is only one ref to this callback. And, of course, GC can to flush it.

[terrakok]

If I save strong link to callback it will be leaked. So, we can save key-callback pairs to stack and flush all newest links when process some result

[terrakok]

But that doesn't get rid of the situation when viewmodel subscribed on result and never received them. And it will be leaked via strong reference on callback when user leaved screen :(

[amatkivskiy]

@terrakok yeah, but maybe we can link callback to a screen that created it? so when you leave the screen that created listener it is clear (but I might be saying some stupid stuff 😄 )

[terrakok]

No. Screen is factory for real views in system (e.g. for fragment). You can use same Screen for several views in chain.

[terrakok]

But it is good direction for thought. May be it is possible associate listeners with current backstack

[amatkivskiy]

@terrakok maybe just a quick (and dirty) fix: what if I remove a listener in the callback body of the listener?

router.setResultListener("terms_accepted") { data ->
     router.removeLisener("terms_accepted")
}

[terrakok]

Quick fix: save link to callback inside viewmodel too

[terrakok]

I've added ResultListenerHandler for cleaning strong link in viewmodel: https://github.com/terrakok/Cicerone/blob/master/sample/src/main/java/com/github/terrakok/cicerone/sample/mvp/animation/profile/ProfilePresenter.kt#L30

[amatkivskiy]

@terrakok Thanks, I will try it today or tomorrow. Thanks for your quick help 👍

[amatkivskiy]

Hi @terrakok, seems that your approach with listener works for me. Thanks for helping 👍 🥇