[core] Fix Updates.reloadAsync crash issue #19538
Conversation
ENG-6714 Crash on Updates.reloadAsync() (reported by RBI)
Original issue report: https://exponent-internal.slack.com/archives/C02VC9BLMQW/p1665498344977319 Here's a reproduction repo: https://github.com/keith-kurak/updates-reload-issue/tree/main And a build that demonstrates the issue (confirmed on Android 12 emulator): https://expo.dev/accounts/keith-kurak/projects/updates-reload-issue/builds/05525183-a505-48b4-86fc-c361f17a46dc How to reproduce:
The app should crash with the |
|
Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines. I've found some issues in your pull request that should be addressed (click on them for more details) 👇
|
# Why same as #19538 but for sdk-46 # How apply the same patch as #19538 and resolve conflict # Test Plan based on https://github.com/keith-kurak/updates-reload-issue + patch-package
…loading on Android (#20063) # Why Closes #20033. # How We can't invalidate `mWrapperDelegateHolders` because view managers won't receive the correct app context. This variable memorizes and updates app context references in the view managers exported from Kotlin. That regression has been introduced in #19538. However, I don't think not clearing `mWrapperDelegateHolders` will affect the `Updates.reloadAsync` behavior. # Test Plan - bare-expo and NCL ✅
…loading on Android (#20063) Closes #20033. We can't invalidate `mWrapperDelegateHolders` because view managers won't receive the correct app context. This variable memorizes and updates app context references in the view managers exported from Kotlin. That regression has been introduced in #19538. However, I don't think not clearing `mWrapperDelegateHolders` will affect the `Updates.reloadAsync` behavior. - bare-expo and NCL ✅ (cherry picked from commit bd9b2b2)
Why
fix
Updates.reloadAsynccrashes on current sdk-46.close ENG-6714
How
following up with #19176, the NativeModulesProxy doesn't be recreated for other cases. the
Updates.reloadAsyncusesReactInstanceManager.recreateReactContextInBackgroundin release build mode and more chance to come across the race condition issue. the ReactApplicationContext in NativeModulesProxy is old and destroyed, theCatalystInstanceImpl.getJSCallInvokerHolderwill throw NPE and cause the crash.basically, the issue should be resolved on main branch because the
KotlinInteropModuleRegistry.shouldBeRecreatedalso checks the liveness of the underlying ReactApplicationContext. i think to check the context from NativeModulesProxy would be more straightforward.i will create another pr for sdk-46 fix.
Test Plan
based on https://github.com/keith-kurak/updates-reload-issue + patch-package
Checklist
expo prebuild& EAS Build (eg: updated a module plugin).