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

fix: fix the potential race condition when dismissing and presentating modal #35705

Closed

Conversation

wood1986
Copy link
Contributor

@wood1986 wood1986 commented Dec 22, 2022

Summary

react-native-screens has a bug about UIViewControllerHierarchyInconsistency when dismissing a react-native <Modal>. Here is the bug software-mansion/react-native-screens#944

After adding dispatch_async block, it solves the issue. But I do not know if this is the right fix.

Here is the example repo https://github.com/wood1986/react-native-modal-crash.

You can revert my last commit wood1986/react-native-modal-crash@86e7bc1 to reproduce the issue. (after revert or hard reset, you need to rerun git clean -xfdq and yarn install)

Changelog

[IOS] [FIXED] - fixed the potential race condition when dismissing and presentating modal

Test Plan

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 22, 2022
@wood1986 wood1986 changed the title fix: fix the potential race condition when dismissing and presentating fix: fix the potential race condition when dismissing and presentating modal Dec 22, 2022
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,467,354 +0
android hermes armeabi-v7a 7,787,208 +0
android hermes x86 8,942,212 +0
android hermes x86_64 8,799,859 +0
android jsc arm64-v8a 9,652,983 +0
android jsc armeabi-v7a 8,386,725 +0
android jsc x86 9,716,974 +0
android jsc x86_64 10,194,432 +0

Base commit: 9669c10
Branch: main

@wood1986 wood1986 force-pushed the fixbug/modal-potential-race-condition branch from a11da0f to 4414f1e Compare December 22, 2022 23:48
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 9669c10
Branch: main

@pull-bot
Copy link

PR build artifact for 4414f1e is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@facebook-github-bot
Copy link
Contributor

@makovkastar has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Jan 3, 2023
@facebook-github-bot
Copy link
Contributor

@makovkastar merged this pull request in e948c79.

kelset pushed a commit that referenced this pull request Jan 19, 2023
…g modal (#35705)

Summary:
`react-native-screens` has a bug about `UIViewControllerHierarchyInconsistency` when dismissing a `react-native` `<Modal>`. Here is the bug software-mansion/react-native-screens#944

After adding `dispatch_async` block, it solves the issue. But I do not know if this is right.

Here is the example repo https://github.com/wood1986/react-native-modal-crash

You can revert my last commit wood1986/react-native-modal-crash@86e7bc1 to reproduce the issue

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[IOS] [FIXED] - fixed the potential race condition when dismissing and presentating modal

Pull Request resolved: #35705

Reviewed By: cipolleschi

Differential Revision: D42253488

Pulled By: makovkastar

fbshipit-source-id: 3e98fa9e719ecdeddeb2a367b0cd364e15136d56
cipolleschi pushed a commit that referenced this pull request Jan 30, 2023
…g modal (#35705)

Summary:
`react-native-screens` has a bug about `UIViewControllerHierarchyInconsistency` when dismissing a `react-native` `<Modal>`. Here is the bug software-mansion/react-native-screens#944

After adding `dispatch_async` block, it solves the issue. But I do not know if this is right.

Here is the example repo https://github.com/wood1986/react-native-modal-crash

You can revert my last commit wood1986/react-native-modal-crash@86e7bc1 to reproduce the issue

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[IOS] [FIXED] - fixed the potential race condition when dismissing and presentating modal

Pull Request resolved: #35705

Reviewed By: cipolleschi

Differential Revision: D42253488

Pulled By: makovkastar

fbshipit-source-id: 3e98fa9e719ecdeddeb2a367b0cd364e15136d56
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
…g modal (facebook#35705)

Summary:
`react-native-screens` has a bug about `UIViewControllerHierarchyInconsistency` when dismissing a `react-native` `<Modal>`. Here is the bug software-mansion/react-native-screens#944

After adding `dispatch_async` block, it solves the issue. But I do not know if this is right.

Here is the example repo https://github.com/wood1986/react-native-modal-crash

You can revert my last commit wood1986/react-native-modal-crash@86e7bc1 to reproduce the issue

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[IOS] [FIXED] - fixed the potential race condition when dismissing and presentating modal

Pull Request resolved: facebook#35705

Reviewed By: cipolleschi

Differential Revision: D42253488

Pulled By: makovkastar

fbshipit-source-id: 3e98fa9e719ecdeddeb2a367b0cd364e15136d56
@cipolleschi cipolleschi mentioned this pull request Oct 11, 2023
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. Merged This PR has been merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants