React-native-navigation: Restarting app via instanceManager.recreateReactContextInBackground leads to crash

[ruslan-bikkinin]

Issue Description

We are working on react-native-code-push module for CodePush and facing troubles investigating this.
Briefly, we are providing API method for "soft" app restart (e.g. without killing application) using RN context restarting mechanism based on instanceManager.recreateReactContextInBackground(); method of ReactInstanceManager class and for some RNN scenarios this approach leads to the app crashes.
I am asking for assistance from you guys to help us to figure out what could be the source of problem here because restarting breaks only for react-native-navigation apps and works like a charm with usual RN app.

Steps to Reproduce / Code Snippets / Screenshots

App.js:

import { AsyncStorage, AppRegistry } from 'react-native';
import { Navigation } from 'react-native-navigation';
import React from 'react';
import { Text } from 'react-native';
import RNRestart from "react-native-restart";

const App = () =>
    <Text onPress={() => RNRestart.Restart()}>
        Touch to run RNRestart.Restart and see the crash
    </Text>
;

const screenName = Math.random().toString();

Navigation.registerComponent(screenName, () => App);
//check what components was registered
console.log("Registered components: " + AppRegistry.getAppKeys());
Navigation.startSingleScreenApp({
    screen: {
        screen: screenName,
    },
    passProps: { },
});

1. Download demo project and run npm i. It uses react-native-restart package that contains pure restart code API extracted from react-native-code-push.
2. Build it using ./gradlew assembleDebug from android directory
3. Install apk on simulator
4. Ensure there is something like this in Logcat logs:

1. Click on button within app.
2. Observe the crash and ensure there is something like this in Logcat logs:

Expected Behavior:
No crash, only one registered application is running.

Actual Behavior:
For some reason RN tries to run old application too instead of running just the new one.

---

Environment

• React Native Navigation version: 1.1.305
• React Native version: 0.50.3
• Platform(s) (iOS, Android, or both?): android
• Device info (Simulator/Device? OS version? Debug/Release?): tested on simulator, android 7.0.0, debug

[die20]

+1

[ruslan-bikkinin]

I want to clarify that the main concern here is

const screenName = Math.random().toString();

Navigation.registerComponent(screenName, () => App);

Could you please guys confirm that this is valid scenario for RNN screen registration? If it is could you please give us any idea how this issue could be resolved?

[ruslan-bikkinin]

Hi guys could you please take a look on it?

[SudoPlz]

If I understood correctly, it seems that, this issue prevents every Android user that implements RNN from code-pushifying their app if they're using AsyncStorage before they register the screens.

[perrosnk]

Any updates on this?

[MrLoh]

I have a similar issue, that the navigation is broken after a codepush restart. Just if I restart the app again afterwards (in my case I first have a singleScreenApp for onboarding and then a tabBasedApp, that's how I encountered this), it seems to work. So I assume that is has something to do with the order in which some code is executed in this case.

[wwwdata]

@MrLoh also have this problem. During development, a code reload just works fine and navigation is working. With Code-Push mandatory updates, the app reloads and then navigation is broken. @ruslan-bikkinin thanks for this plugin, with it I could verify that it is broken after the reload.

And of course when I am connected to the debugger and use the react-native-restart plugin, everything is also just working fine and navigation works...

@MrLoh could you find out anything that helps?

We are also using AsyncStorage to persist our redux state.

[wwwdata]

Hm, I found out that when I do the reload, switch to a singleScreenApp and then switch back to my normal authenticated tabBasedApp, everything is working again ...

So maybe there is some global stuff that needs to be re-initialized or something?

[MrLoh]

As a workaround, I am currently always starting an empty (just a single white screen) singleScreenApp before I start the main app. This is a really ugly workaround, but at least it works.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you believe the issue is still relevant, please test on the latest version and report back. Thank you for your contributions.

[MrLoh]

I moved away from react-native-navigation because of this issue, it's probably not been resolved yet.

[ericketts]

Can confirm its not been resolved, presently trying to figure out a workaround...

based on my cursory exploration, it appears the crux of the issue is: the NavigationActivity presenting the view that triggers the restart has a ReactRootView registered with the ReactInstanceManager#mAttachedRootViews; when the restart is triggered, the React JS context gets recreated, and as part of that process, an attempt is made to re-mount all the old ReactRootViews, where this remount invariably attempts to call the JS function AppRegistry#runApplication() from Java, across the bridge.

However, because the context hasn't actually been fully recreated, the JS bundle has not been parsed & run, and AppRegistry doesn't exist yet; the app crash with an invariant violation.

To be quite honest I'm not fully sure this is correct, and I don't yet understand why this isn't an issue when RNN is not integrated -- this is what I'm attempting to determine now.

My guess at a fix would be removing the dying ReactRootView from the ReactInstanceManager collection, which usually occurs in NavigationActivity.onDestroy() (with a call to destroyLayouts() which in turn calls layout.destroy(), which eventually does remove it from the mAttachedRootViews collection). However I cannot find a good way to detect that a reload is occurring from within the NavigationActivity, so layout.destroy() is not called until the NavigationActivity is dismissed & destroyed, by which point its too late (ie invariant trigger a red screen in debug, a crash in prod).

If anyone has any ideas for how we might be able to detect the call to recreateReactContextInBackground from within the NavigationActivity, or knows that something I've said here is wrong, I'd be most obliged for any assistance, as I'm getting to the point of wheel spinning...

[ericketts]

Quick follow up to my (painfully long) previous comment, if I hack apart the restart functionality and use getCurrentActivity() to manually call destroyLayouts() on the presented NavigationActivity immediately preceding the call to ReactInstanceManager.recreateReactContextInBackground(), I do not see the redbox nor error in logcat, which seems to confirm my suspicions in the previous post.

Now how to do this without the hack is the billion dollar movie...

[ericketts]

FYI, I put together a minimal repro here.

[ericketts]

fwiw the PR I submitted that's linked above seems to fix this bug for me... I'm a bit worried about potential race condition but so far I have yet to see one arise.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you believe the issue is still relevant, please test on the latest version and report back. Thank you for your contributions.

[stale[bot]]

The issue has been closed for inactivity.

[terreb]

Ok, do I assume correctly that code push can not be used with react native navigation v.1 on Android? Does it make sense to upgrade to v2 or the issue would still persist there and better to move to react-navigation (what I would like to avoid)?

[SudoPlz]

@terreb We've been using codepush and RNN v1 for months now without issues.
You just have to use AsyncStorage AFTER you register your RNN screens.

[ericketts]

@SudoPlz are you using codepush configured with ON_NEXT_RESTART? because as far as I can tell, every other mode but that one uses the context restart that causes a crash with RNNv1 when building for prod

[SudoPlz]

Nope, I'm using IMMEDIATE and no I don't get a crash.

[terreb]

@SudoPlz, I tried both before and after and in both cases I have a crash if not using ON_NEXT_RESTART install mode. Would you mind to share a minimum working repo on Android with RNN 1 and code push?

[SudoPlz]

@terreb Unfortunately I'm too busy to do that at the moment, but we don't do anything extra-ordinary, we're just registering the screens first, and THEN use AsyncStorage.

[ericketts]

@terreb are you red screen/crashing on android when restarting?

try implementing what I did in this PR and see if it works for you then. (I'm really not sure how its working for sudoplz, there's a bug on android where it attempts to reattach views to a dead JS context which crashes when built for prod and red screens in dev mode).

[terreb]

@ericketts, no I don't get a red screen, the app just closes when crashing. I'm testing on a Genymotion emulator, it might behave differently on a real device but I don't have any around.

Yes I saw your PR and wanted to thank you for your efforts. However I hesitated with trying it because most likely it won't be integrated in an official version. I decided to migrate to v2 and see if it works there. Thank you again.

[ericketts]

@terreb yeah for sure, we just migrated to v2 and it so far doesn't seem to have the numerous issues experienced on android in v1... also as far as crash vs red screen, what is a red screen when building for dev will be a runtime crash when built for prod, so you're really getting both most likely... the red screen helps explain the issue so it can be useful to build in dev mode for that reason!

[terreb]

@ericketts, ok I got you and I know what the red screen is:) The thing is it works in dev mode and not crashing. In this case, if I understood correctly, code push uses RN bundler to load the package. I get the crash only in production, so no red screens there:)

[terreb]

Migrated to RNN v2, the same issue, this is very frustrating:(

[ericketts]

Its curious because it appears as though there is code in the V2 rearch specifically to handle the issue that was causing this crash bug, but it apparently isn't working as like @terreb I still see this issue arise in V2. unfortunate that this issue was closed, it is not at all solved.

[ujwal-setlur]

@ericketts can your PR be adapted to V2? I have this issue on V2 as well...

[ericketts]

@ujwal-setlur I had a quick look at the internals of android V2, and it looks sufficiently different that I can't simply graft the previous fix onto the new code... however the crash is affecting the app we're developing presently, so I would not be surprised if there comes a time where I will need to figure out a fix for V2, at which point I would definitely (at least attempt to) get it merged back in to the main codebase.

[afilp]

@ericketts Our app crashes selectively on code push (most devices crash, others not). We use this checkFrequency: codePush.CheckFrequency.ON_APP_RESUME, and we really want to keep it that way.

Is there any solution to this now? We have react-native 0.60

[bitpit]

@afilp it really depends on why exactly things are fucking up, I'd have to investigate your app and stack traces and all that jazz to answer with any certainty. I can tell you I had to whack-a-mole about 2 or 3 different reasons (sounds worse than it really was since that was between v2 and v3)

now if you're hiring... 😄

[SudoPlz]

@bitpit are you the same person as @ericketts ?
(There's an open full time position for someone with your skills in our company, please dm me if you're interested)

[JB-CHAUVIN]

Hello, I still have the problem.

Why is this issue closed?

[JB-CHAUVIN]

Can we re-open this issue ? I don't see any awnsers !