Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[modules][Android] Fix JSCRuntime destroyed with a dangling API object #19487

Merged
merged 2 commits into from
Oct 11, 2022

Conversation

lukmccall
Copy link
Contributor

Why

Closes #19221.

How

In the current memory modal, all of our JSI references are cleared when GC runs through java unused objects. It turns out that is not the best flow, because JSC has an assertion that checks if all references were cleared. So we have to remove dangling references before the JSC runtime fully deallocates.

Test Plan

  • bare-expo with JSC and Heremes ✅

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Oct 10, 2022
Copy link
Contributor

@Kudo Kudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 nice fix 🔥

Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
Co-authored-by: Kudo Chien <kudo@expo.dev>
@lukmccall lukmccall merged commit 33ae33e into main Oct 11, 2022
@lukmccall lukmccall deleted the @lukmccall/core/fix-crases-on-jsc branch October 11, 2022 07:56
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Oct 11, 2022
@gasolin
Copy link

gasolin commented Oct 13, 2022

thanks for the fix!
will the fix include in EXPO 46?

Kudo pushed a commit that referenced this pull request Oct 13, 2022
…ct` (#19487)

Closes #19221.

In the current memory modal, all of our JSI references are cleared when GC runs through java unused objects. It turns out that is not the best flow, because JSC has an assertion that checks if all references were cleared. So we have to remove dangling references before the JSC runtime fully deallocates.

- bare-expo with JSC and Heremes ✅

(cherry picked from commit 33ae33e)
@Kudo
Copy link
Contributor

Kudo commented Oct 14, 2022

@gasolin the expo@46.0.16 should have this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expo bare-workflow app is crashing when expo-localization is imported
5 participants