Skip to content

Defer HostTarget destruction until after the instance has been unregistered#44767

Closed
motiz88 wants to merge 2 commits into
facebook:mainfrom
motiz88:export-D58031215
Closed

Defer HostTarget destruction until after the instance has been unregistered#44767
motiz88 wants to merge 2 commits into
facebook:mainfrom
motiz88:export-D58031215

Conversation

@motiz88
Copy link
Copy Markdown
Contributor

@motiz88 motiz88 commented Jun 3, 2024

Summary:
Changelog: [Internal]

Fixes a lifecycle bug in both the Bridge (com.facebook.react.bridge) and Bridgeless (com.facebook.react.runtime) integrations of Fusebox in React Native Android, whereby HostTarget::unregisterInstance gets called after the HostTarget has been destroyed.

The solution consists of two parts:

  1. If a ReactHost / InstanceManager is asked to destroy itself while it contains no active ReactInstance / ReactContext, we destroy the HostTarget immediately.
  2. Otherwise, if there is a live ReactInstance / ReactContext that has yet to be destroyed, we wait for that to happen before destroying the HostTarget. In practice, we do this by checking for the BEFORE_CREATE ( = Host destroyed) lifecycle state every time we destroy a ReactInstance / ReactContext.

Differential Revision: D58031215

motiz88 added 2 commits June 3, 2024 10:20
Summary:
Changelog: [Internal]

There is currently a lifecycle bug in both the Bridge (`com.facebook.react.bridge`) and Bridgeless (`com.facebook.react.runtime`) integrations of Fusebox in React Native Android, whereby `HostTarget::unregisterInstance` gets called after the `HostTarget` has been destroyed. This manifests as a handful of related C++ crashes depending on the exact circumstances and build flags.

This diff makes the bug trigger a Java assertion instead of a C++ crash for ease of debugging. The next diff in the stack will actually fix the lifecycle issue.

Differential Revision: D58031217
…stered

Summary:
Changelog: [Internal]

Fixes a lifecycle bug in both the Bridge (`com.facebook.react.bridge`) and Bridgeless (`com.facebook.react.runtime`) integrations of Fusebox in React Native Android, whereby `HostTarget::unregisterInstance` gets called after the `HostTarget` has been destroyed.

The solution consists of two parts:

1. If a ReactHost / InstanceManager is asked to destroy itself while it contains no active ReactInstance / ReactContext, we destroy the `HostTarget` immediately.
2. Otherwise, if there *is* a live ReactInstance / ReactContext that has yet to be destroyed, we wait for that to happen before destroying the `HostTarget`. In practice, we do this by checking for the BEFORE_CREATE ( = Host destroyed) lifecycle state every time we destroy a ReactInstance / ReactContext.

Differential Revision: D58031215
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels Jun 3, 2024
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D58031215

@analysis-bot
Copy link
Copy Markdown

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 19,576,594 -9
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 22,946,155 +4
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: a6a7cdf
Branch: main

@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request has been merged in bbf8a87.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Jun 6, 2024
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 6, 2024

This pull request was successfully merged by @motiz88 in bbf8a87.

When will my fix make it into a release? | How to file a pick request?

kosmydel pushed a commit to kosmydel/react-native that referenced this pull request Jun 11, 2024
…stered (facebook#44767)

Summary:
Pull Request resolved: facebook#44767

Changelog: [Internal]

Fixes a lifecycle bug in both the Bridge (`com.facebook.react.bridge`) and Bridgeless (`com.facebook.react.runtime`) integrations of Fusebox in React Native Android, whereby `HostTarget::unregisterInstance` gets called after the `HostTarget` has been destroyed.

The solution consists of two parts:

1. If a ReactHost / InstanceManager is asked to destroy itself while it contains no active ReactInstance / ReactContext, we destroy the `HostTarget` immediately.
2. Otherwise, if there *is* a live ReactInstance / ReactContext that has yet to be destroyed, we wait for that to happen before destroying the `HostTarget`. In practice, we do this by checking for the BEFORE_CREATE ( = Host destroyed) lifecycle state every time we destroy a ReactInstance / ReactContext.

Reviewed By: javache

Differential Revision: D58031215

fbshipit-source-id: 321c73e85afd17a1b38c63f73aee5ebb59c00686
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants