Occasional "IllegalStateException: Restarter must be created only during owner's initialization stage"

[rjrjr]

I can't reproduce this, but we're seeing a steady stream in production.

java.lang.RuntimeException: Unable to start activity ComponentInfo{com.squareup.apos/com.squareup.ui.main.MainActivity}: java.lang.IllegalStateException: Restarter must be created only during owner's initialization stage
        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3031)
        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3166)
        at android.app.ActivityThread.handleRelaunchActivityInner(ActivityThread.java:4929)
        at android.app.ActivityThread.handleRelaunchActivity(ActivityThread.java:4838)
        at android.app.servertransaction.ActivityRelaunchItem.execute(ActivityRelaunchItem.java:69)
        at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:108)
        at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:68)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1883)
        at android.os.Handler.dispatchMessage(Handler.java:106)
        at android.os.Looper.loop(Looper.java:193)
        at android.app.ActivityThread.main(ActivityThread.java:6971)
        at java.lang.reflect.Method.invoke(Method.java:-2)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:865)

Caused by: java.lang.IllegalStateException: Restarter must be created only during owner's initialization stage
        at androidx.savedstate.SavedStateRegistryController.performRestore(SavedStateRegistryController.java:58)
        at com.squareup.workflow1.ui.backstack.StateRegistryAggregator.restoreRegistryControllerIfReady(StateRegistryAggregator.kt:195)
        at com.squareup.workflow1.ui.backstack.ViewStateCache$registryHolder$2.invoke(ViewStateCache.kt:54)
        at com.squareup.workflow1.ui.backstack.ViewStateCache$registryHolder$2.invoke(ViewStateCache.kt:52)
        at com.squareup.workflow1.ui.backstack.StateRegistryAggregator.restoreFromBundle(StateRegistryAggregator.kt:228)
        at com.squareup.workflow1.ui.backstack.StateRegistryAggregator.access$restoreFromBundle(StateRegistryAggregator.kt:39)
        at com.squareup.workflow1.ui.backstack.StateRegistryAggregator$lifecycleObserver$1.onStateChanged(StateRegistryAggregator.kt:87)
        at androidx.lifecycle.LifecycleRegistry$ObserverWithState.dispatchEvent(LifecycleRegistry.java:354)
        at androidx.lifecycle.LifecycleRegistry.forwardPass(LifecycleRegistry.java:265)
        at androidx.lifecycle.LifecycleRegistry.sync(LifecycleRegistry.java:307)
        at androidx.lifecycle.LifecycleRegistry.moveToState(LifecycleRegistry.java:148)
        at androidx.lifecycle.LifecycleRegistry.handleLifecycleEvent(LifecycleRegistry.java:134)
        at androidx.lifecycle.ReportFragment.dispatch(ReportFragment.java:68)
        at androidx.lifecycle.ReportFragment.dispatch(ReportFragment.java:144)
        at androidx.lifecycle.ReportFragment.onActivityCreated(ReportFragment.java:102)
        at android.app.Fragment.performActivityCreated(Fragment.java:2531)
        at android.app.FragmentManagerImpl.moveToState(FragmentManager.java:1318)
        at android.app.FragmentManagerImpl.moveFragmentToExpectedState(FragmentManager.java:1576)
        at android.app.FragmentManagerImpl.moveToState(FragmentManager.java:1637)
        at android.app.FragmentManagerImpl.dispatchMoveToState(FragmentManager.java:3046)
        at android.app.FragmentManagerImpl.dispatchActivityCreated(FragmentManager.java:2998)
        at android.app.FragmentController.dispatchActivityCreated(FragmentController.java:182)
        at android.app.Activity.performCreate(Activity.java:7249)
        at android.app.Activity.performCreate(Activity.java:7233)
        at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1287)
        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3011)
        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3166)
        at android.app.ActivityThread.handleRelaunchActivityInner(ActivityThread.java:4929)
        at android.app.ActivityThread.handleRelaunchActivity(ActivityThread.java:4838)
        at android.app.servertransaction.ActivityRelaunchItem.execute(ActivityRelaunchItem.java:69)
        at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:108)
        at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:68)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1883)
        at android.os.Handler.dispatchMessage(Handler.java:106)
        at android.os.Looper.loop(Looper.java:193)
        at android.app.ActivityThread.main(ActivityThread.java:6971)
        at java.lang.reflect.Method.invoke(Method.java:-2)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:865)

[rjrjr]

Trying and failing to repro by doing awful things to the espresso tests that drive the related code, which is this call in ViewStateCache:

    onRestored = { aggregator ->
      currentOwner?.let { owner ->
        aggregator.restoreRegistryControllerIfReady(owner.key, owner.controller)
      }
    }

In the wild we're crashing because currentOwner has moved passed INITIALIZED. So far I can't figure out how to make that happen.

In theory the fix is as simple as this:

    onRestored = { aggregator ->
      currentOwner?.let { owner ->
        if (owner.lifecycle.currentState == INITIALIZED) {
          aggregator.restoreRegistryControllerIfReady(owner.key, owner.controller)
        }
      }
    }

That doesn't break any tests, and the tests definitely cover this extensively. But I'd much prefer being able to make the bad behavior happen before making this change.

Other notes:

• It's happening in all versions of Android
• Android 12 bypasses androidx.lifecycle.ReportFragment and uses some lifecycle hooks instead, but we've seen instances that way too
• Both phone and tablet
• Happens all over the app
• Always either immediately after a handoff between activity instances, or within seconds of it
• Smells like a config change, but I've never seen it associated with an orientation change.

[rjrjr]

I wonder if it could be a race between the new activity spinning up and ViewStateCache#update in the previous activity? That method updates currentOwner. Don't see how we'd bleed from the old world to the new, though. Unless Android is bleeding for us?

[rjrjr]

Makes me wonder if ActivityScenario.recreate() is too well behaved compared to whatever happens IRL.

[rjrjr]

Tried getting more realistic this way:

      scenario.onActivity {
        it.requestedOrientation =
          if (i % 2 == 0) SCREEN_ORIENTATION_PORTRAIT else SCREEN_ORIENTATION_LANDSCAPE
      }

Behavior is still flawless.

[rjrjr]

Looking harder at this portion of the stack trace:

at androidx.savedstate.SavedStateRegistryController.performRestore(SavedStateRegistryController.java:58)
at …StateRegistryAggregator.restoreRegistryControllerIfReady(StateRegistryAggregator.kt:195)
at …ViewStateCache$registryHolder$2.invoke(ViewStateCache.kt:54)
at …ViewStateCache$registryHolder$2.invoke(ViewStateCache.kt:52)
at …StateRegistryAggregator.restoreFromBundle(StateRegistryAggregator.kt:228)
at …StateRegistryAggregator.access$restoreFromBundle(StateRegistryAggregator.kt:39)
at …StateRegistryAggregator$lifecycleObserver$1.onStateChanged(StateRegistryAggregator.kt:87)
at androidx.lifecycle.LifecycleRegistry$ObserverWithState.dispatchEvent(LifecycleRegistry.java:354)

• StateRegistryAggregator#lifecycleObserver receives ON_CREATE. We know this because it will throw otherwise. It should only be possible to receive that event while in the INITIALIZED state.
• From there, StateRegistryAggregator.restoreFromBundle calls the onRestored function that the owning ViewStateCache provided.
• We crash because ViewStateCache.currentOwner has already left the INITIALIZED state.

So StateRegistryAggregator must be observing something other than ViewStateCache.currentOwner; or Android is delivering us redundant events. I don't really believe the latter because restoreFromBundle would short circuit on the second call since the second call to consumeRestoredStateForKey should return null.

[rjrjr]

The StateRegistryAggregator is observing whatever SavedStateRegistryOwner.lifecycleOwner was handed to ViewStateCache.attachToParentRegistry by BackStackContainer.onAttachedToWindow.

Meanwhile, ViewStateCache.currentOwner, the thing that advanced out of INITIALIZED unexpectedly, gets updated on every call to BackStackContainer#update > ViewStateCache#update.

update should be called first, before onAttachedToWindow

If you drill down from ViewStateCache#update, the new currentOwner comes from a call to WorkflowLifecycleOwner.get. The corresponding WorkflowLifecycleOwner.installOn call probably came from ModalContainer -- unless we're accidentally wrapping a BackStackContainer in a BackStackContainer. But I think we'd crash sooner if that was the case.

[rjrjr]

Wouldn't it be funny if these crashes were due to the racy behavior of that WeakReference that I removed recently? #560

Putting a pin on this to see if these crashes evaporate as that fix reaches production.

[NickvanDyke]

I can recreate this reliably in my app. It's occurring on my settings page, which has a dark mode setting that calls activity.recreate() when it changes, so that the theme updates. My PreferencesLayoutRunner just adds and removes a PreferencesCompatFragment to its layout's FragmentContainerView, so I can take advantage of Android's built in preference stuff. Which might be hacky? Anyway, here's the source:

preferences_screen.xml:

<?xml version="1.0" encoding="utf-8"?>
<FrameLayout xmlns:android="http://schemas.android.com/apk/res/android"
    android:orientation="vertical"
    android:layout_width="match_parent"
    android:layout_height="match_parent">

    <androidx.fragment.app.FragmentContainerView
        android:id="@+id/preferences_fragment"
        android:layout_width="match_parent"
        android:layout_height="match_parent" />

</FrameLayout>

PreferencesLayoutRunner:

class PreferencesLayoutRunner(private val binding: PreferencesScreenBinding) : LayoutRunner<PreferencesScreen> {

    init {
        // Note: crash was still occurring before wrapping this in doOnAttach
        binding.root.doOnAttach {
            val prefsFragment = PreferencesFragment()
            binding.activity.supportFragmentManager.beginTransaction()
                .replace(R.id.preferences_fragment, prefsFragment)
                .commit()

            // Otherwise it seems to stick around and leak. Have to queue this here otherwise it'll
            // execute immediately since we're detached in init.
            // Note: crash was still occurring before adding this block.
            binding.root.doOnDetach {
                binding.activity.supportFragmentManager.takeUnless { it.isDestroyed }
                    ?.beginTransaction()
                    ?.remove(prefsFragment)
                    ?.commit()
            }
        }
    }

    override fun showRendering(rendering: PreferencesScreen, viewEnvironment: ViewEnvironment) {
    }

    companion object : ViewFactory<PreferencesScreen> by bind(PreferencesScreenBinding::inflate, ::PreferencesLayoutRunner)
}

PreferencesFragment (irrelevant code omitted):

class PreferencesFragment : PreferenceFragmentCompat() {

    override fun onCreatePreferences(savedInstanceState: Bundle?, rootKey: String?) {
        addPreferencesFromResource(R.xml.preferences)

        findPreference<Preference>(getString(R.string.dark_mode_key))!!.apply {
            setOnPreferenceChangeListener { _, newValue ->
                // Location permission is required for auto time
                if (newValue == "Auto" && ContextCompat.checkSelfPermission(requireContext(), Manifest.permission.ACCESS_FINE_LOCATION) != PackageManager.PERMISSION_GRANTED) {
                    AlertDialog.Builder(requireContext())
                        .setMessage(requireView().context!!.getString(R.string.location_permission_rationale))
                        .setPositiveButton("OK") { dialog, _ ->
                            requireActivity().lifecycleScope.launch {
                                Peko.requestPermissionsAsync(requireActivity(), Manifest.permission.ACCESS_FINE_LOCATION)
                                // Recreate regardless of granted, since we still changed the night mode setting.
                                // Nullable activity because the activity might be recreated while the permission
                                // dialog is visible. In that case, don't need to recreate it ourselves.
                                activity?.recreate()
                            }
                            dialog.dismiss()
                        }
                        .show()
                } else {
                    // TODO crashing. https://github.com/square/workflow-kotlin/issues/570
                    requireActivity().recreate()
                }
                false
            }
        }
    }
}

It doesn't crash if I remove activity.recreate() and trigger a manual recreation via rotation. I thought the delay between the preference getting set and triggering the manual recreation might have helped, so I wrapped activity.recreate() in Handler.postDelayed(1000), but still crashed.

It used to work fine, and I believe started crashing when I updated AndroidX Appcompat version to fix another bug. https://stackoverflow.com/questions/56539251/backstack-management-restarter-must-be-created-only-during-owners-initializat seems relevant.

Hopefully that's helpful, let me know if any other info would be relevant.

[rjrjr]

@NickvanDyke Belated thanks for this! The issue hasn't reached the top of my stack yet, but it isn't far down. LMK if it's something you need looked at more aggressively.

[NickvanDyke]

@rjrjr no prob! It won't be needed for us until a month or prolly two until now, so you're probably good to take your time. I appreciate the consideration!