facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
118.77k stars 24.29k forks source link

Android ReactActivity Destroy Prevents Successful OnActivityResult Calls #30277

Open zsweigart opened 3 years ago

zsweigart commented 3 years ago

Description

If a native module opens an activity that redirects out of the application and the application is killed in the background by the Android OS, the react context is lost and the native module's activity is unable to call onActivityResult.

If the redirect from the third party app is back to the activity in the native module, the ReactActivity is not created at this point. When the native activity then sets result and finishes to return back to the ReactActivity the ReactActivity has to start up.

The React Native startup flow is:

  1. ReactActivity#onCreate
  2. ReactActivityDelegate#onCreate
  3. ReactActivityDelegate#loadApp
  4. ReactDelegate#loadApp
  5. ReactRootView#startReactApplication
  6. ReactInstanceManager#createReactContextInBackground
  7. ReactInstanceManager#recreateReactContextInBackgroundInner
  8. ReactInstanceManager#recreateReactContextInBackgroundFromBundleLoader
  9. ReactInstanceManager#recreateReactContextInBackground
  10. ReactInstanceManager#runCreateReactContextOnNewThread

At this point ReactInstanceManager#setupReactContext is called on a background thread.

Meanwhile, ReactActivity#onActivityResult is called. In the Android activity lifecycle onActivityResult is called even before onResume (for reference. The calls from ReactActivity#onActivityResult pass through a few layers and end up at ReactInstanceManager#onActivityResult which checks if the ReactContext is null

  @ThreadConfined(UI)
  public void onActivityResult(Activity activity, int requestCode, int resultCode, Intent data) {
    ReactContext currentContext = getCurrentReactContext();
    if (currentContext != null) {
      currentContext.onActivityResult(activity, requestCode, resultCode, data);
    }
  }

Because the context is being built on a background thread there is no guarantee that currentContext will be initialized at this point. Furthermore ReactInstanceManager#createReactContext is a fairly intensive function so it is actually very likely that currentContext is null.

This means that ReactContext#onActivityResult is not called and therefore, the data cannot be passed back to the ActivityEventListener which in my sample is the ReactContextBaseJavaModule.

React Native version:

System:
    OS: macOS 10.15.7
    CPU: (12) x64 Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz
    Memory: 118.90 MB / 32.00 GB
    Shell: 3.2.57 - /bin/bash
  Binaries:
    Node: 13.14.0 - ~/.nvm/versions/node/v13.14.0/bin/node
    Yarn: 1.22.5 - /usr/local/bin/yarn
    npm: 6.14.4 - ~/.nvm/versions/node/v13.14.0/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  Managers:
    CocoaPods: 1.9.3 - /usr/local/bin/pod
  SDKs:
    iOS SDK:
      Platforms: iOS 13.6, DriverKit 19.0, macOS 10.15, tvOS 13.4, watchOS 6.2
    Android SDK:
      API Levels: 19, 23, 26, 27, 28, 29
      Build Tools: 27.0.3, 28.0.3, 29.0.0, 29.0.1, 29.0.2
      System Images: android-21 | Google APIs Intel x86 Atom, android-23 | Google APIs Intel x86 Atom, android-26 | Google Play Intel x86 Atom, android-28 | Google APIs Intel x86 Atom, android-29 | Google Play Intel x86 Atom, android-Q | Android TV Intel x86 Atom
      Android NDK: Not Found
  IDEs:
    Android Studio: 4.0 AI-193.6911.18.40.6626763
    Xcode: 11.6/11E708 - /usr/bin/xcodebuild
  Languages:
    Java: 1.8.0_265 - /usr/bin/javac
    Python: 2.7.16 - /usr/bin/python
  npmPackages:
    @react-native-community/cli: Not Found
    react: 16.13.1 => 16.13.1 
    react-native: 0.63.3 => 0.63.3 
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

Steps To Reproduce

  1. Clone
  2. https://github.com/zsweigart/react-native-crash
  3. cd into the directory and run react-native run-android
  4. On your device or emulator go to developer options
  5. Turn Do Not Keep Activities to On
  6. Set Background Process Limit to No background processes
  7. Click Open Activity
  8. Click Go to Web -> This should kill the original process in the background due to your developer settings; however, on some emulators I have noticed it does not. One way to guarantee this is to run adb shell am kill com.react_native_crash which will simulate a system process kill. Note you cannot kill the process by swiping it away because the OS will lose the information about the backstack.
  9. Next run adb shell am start -n com.react_native_crash/.RedirectActivity -d "redirect://crash"
  10. Click Return Result

Expected Results

After these steps in the console you should see

 LOG      RESULT
 LOG      Completed

which would indicate the onActivityResult was called properly and the callback to the js code was executed.

Snack, code example, screenshot, or link to a repository:

https://github.com/zsweigart/react-native-crash

zandrei commented 3 years ago

Are there any updates on this issue? This is a problem with low memory devices as well when the main application launches other activities (like launching camera) and then the Android OS kills the activity that launched the camera along with the ReactContext.

zandrei commented 3 years ago

@zsweigart Did you find any workaround for this issue? Maybe provide an own ReactInstanceManager implementation?

zsweigart commented 3 years ago

We have not worked on finding a work around and are currently accepting the rate errors until this is fixed

HugoGresse commented 3 years ago

For the sake of cross-referencing most related issues from other modules and working on a workaround, I've created a repo: https://github.com/HugoGresse/react-native-issue-30277/blob/main/README.md

HugoGresse commented 3 years ago

Workaround for react-native-image-crop-picker here

LiamBateman commented 3 years ago

What changed to cause this? Is it a ReactNative change or an Android SDK update?

VictorAugustoDn commented 3 years ago

I can't believe there is no update on this issue.

stale[bot] commented 2 years ago

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

HugoGresse commented 2 years ago

This issue needs attention, it impact a lot of apps through many libraries.

BLOCKMATERIAL commented 2 years ago

Same problem (image-crop-picker).

avijitacharjee commented 2 years ago

Facing the same issue... I think this issue needs attention.

omdxp commented 2 years ago

This issue needs attention, facing the same thing!

abdulqadir-jugnu commented 2 years ago

Any one with a solution?

chathuraSupun commented 2 years ago

Currently facing the same issue with react-native-app-auth https://github.com/FormidableLabs/react-native-app-auth/issues/773

sladek-jan commented 1 year ago

As a workaround, I can imagine one would use a Handler on the main thread (which is guaranteed here anyway), poll whether reactContext is still null and give up after a few retries (trying to cover as many crashes as possible but not waiting too long). Obviously, one would need to patch react-native using patch-package to achieve that, but it would work across libraries. It would be good to only keep a WeakReference to the activity to not leak it, unless this would be the last reference to it, in which case it'd be necessary to keep it around for those few seconds until the React context initializes.

huanguolin commented 1 year ago

Facing the same issue... I think this issue needs attention.

github-actions[bot] commented 9 months ago

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

huanguolin commented 9 months ago

This issue needs attention!

tamdvyounetgroup commented 5 months ago

any solutions?

sanduluca commented 2 weeks ago

Ca we have someones attention to this ? @react-native-bot @cipolleschi @cortinico

cipolleschi commented 2 weeks ago

can someone create a clear reproducer using this template? The original issue is pretty old and the latest reproducer is from a year ago.

HugoGresse commented 2 weeks ago

I'm unsure it will help as I find it was dependent of the device, using the dev settings to not keep the activities does not seems to have the same effect. I can send you a Xiaomi Redmi 7A if needed More info on https://github.com/HugoGresse/react-native-issue-30277/tree/main