Crashing with `java.lang.IllegalStateException: no event up from DESTROYED` about voyager

[DevSrSouza]

@L-Andrade release the fix in 1.1.0-alpha02

[DevSrSouza]

If you could make a reproduceable of this would be awesome!

@Syer10 @adrielcafe

It seems that we should check the current state before emiting the stopEvents, aparently, it is already DESTROYED , I really would want to be able to reproduce to validate a proper fix the code with a workaround :S but maybe a trycatch here or validate the current state would fix this.

private fun emitOnStopEvents() {
        stopEvents.forEach {
            lifecycle.handleLifecycleEvent(it)
        }
    }

[L-Andrade]

@DevSrSouza Unfortunately I haven't managed to reproduce it yet 😞

Let me know if you have anything in mind that I can try to replicate it! I'll give it some more tries next week as well once I'm back to work

[L-Andrade]

I believe this is due to a DataBinding component we have inside a Screen. We were setting the lifecycleOwner of this DataBinding component to the LocalLifecycleOwner (which is an AndroidScreenLifecycleOwner). Inside the generated DataBinding code, there are multiple things happening, including observers being attached to the lifecycle.

This issue is similar, albeit from a different context entirely: https://issuetracker.google.com/issues/184664896

I'm not sure how you folks want to deal with this, or if you want to deal with it at all. In that issue, they just checked the state before emitting it: https://android-review.googlesource.com/c/platform/frameworks/support/+/1673541/4/car/app/app/src/main/java/androidx/car/app/Screen.java#321

The way I managed to reproduce the issue was to make the DataBinding component appear, disappear, and then appear again. I needed some code changes to be able to reproduce it exactly, so I'm not entirely sure it is the only issue.

For now, I've removed the LifecycleOwner attribution from the DataBinding component. I believe it's used more when there's observers/collectors in the DataBinding component, but in this one we don't use it, and it is still working as expected, so it shouldn't be a big issue.

The build with this change will ship on the 15th of January. I'll ping here if the issue appears to be fixed. I might try to replicate it later in an empty project. Feel free to try it yourself as well, of course!

[L-Andrade]

@DevSrSouza Here's a project that replicates the issue:

• MainActivity.kt:

class MainActivity : ComponentActivity() {
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContent {
            VoyagerDemoTheme {
                SlideNavigator(InitialScreen())
            }
        }
    }
}

@Composable
private fun SlideNavigator(navScreen: Screen) {
    Navigator(navScreen) { navigator ->
        SlideTransition(navigator)
    }
}


class InitialScreen : Screen {
    override val key: ScreenKey = uniqueScreenKey

    @Composable
    override fun Content() {
        val navigator = LocalNavigator.currentOrThrow
        Button(onClick = { navigator.push(Screen1()) }) {
            Text(text = "Push new Screen")
        }
    }
}

class Screen1 : Screen {
    override val key: ScreenKey = uniqueScreenKey

    @Composable
    override fun Content() {
        Box(modifier = Modifier.fillMaxSize()) {
            var show by remember { mutableStateOf(false) }
            LaunchedEffect(key1 = Unit) {
                while (isActive) {
                    delay(250)
                    show = !show
                }
            }
            if (!show) {
                CircularProgressIndicator(modifier = Modifier.align(Alignment.Center))
            } else {
                val lifecycleOwner = LocalLifecycleOwner.current
                AndroidViewBinding(
                    factory = { inflater, parent, attachToParent ->
                        LayoutDataBindingBinding.inflate(inflater, parent, attachToParent)
                            .apply {
                                this.lifecycleOwner = lifecycleOwner
                            }
                    },
                ) {
                    this.text = "Show: $show"
                }
            }
        }
    }
}

• layout_data_binding.xml

<?xml version="1.0" encoding="utf-8"?>
<layout xmlns:android="http://schemas.android.com/apk/res/android">

    <data>
        <variable
            name="text"
            type="String" />
    </data>
    <TextView
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        android:text="@{text}" />

</layout>

• Dependencies:

dependencies {
    implementation("androidx.core:core-ktx:1.9.0")
    implementation("androidx.lifecycle:lifecycle-runtime-ktx:2.6.1")
    implementation("androidx.activity:activity-compose:1.7.2")
    implementation(platform("androidx.compose:compose-bom:2023.10.01"))
    implementation("androidx.compose.ui:ui")
    implementation("androidx.compose.ui:ui-graphics")
    implementation("androidx.compose.ui:ui-tooling-preview")
    implementation("androidx.compose.material3:material3")
    debugImplementation("androidx.compose.ui:ui-tooling")
    debugImplementation("androidx.compose.ui:ui-test-manifest")
    val voyagerVersion = "1.0.0"
    implementation("cafe.adriel.voyager:voyager-navigator:$voyagerVersion")
    implementation("cafe.adriel.voyager:voyager-transitions:$voyagerVersion")

    implementation("androidx.compose.ui:ui-viewbinding")
}

Once setup, you just need to navigate to the next screen and once it hides and shows a few times, navigate back. The crash occurs when navigating back.

Also managed to replicate the issue with this instead of the AndroidViewBinding:

val lifecycleOwner = LocalLifecycleOwner.current
LaunchedEffect(key1 = Unit) {
    lifecycleOwner.lifecycle.addObserver(object : DefaultLifecycleObserver {})
}

When using a DisposableEffect and properly removing the observer in onDispose, it did not crash. So it might mean that the DataBinding component is not properly clearing the observer?

Nevertheless, it might be nice if Voyager would be defensive here and only emit when it's supposed to, instead of creating these crashes. They're pretty hard to debug from an app perspective (even if there might be a leak on the app's end).