-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Call assert in SingleInstanceChecker
only in example apps
#4316
Conversation
Can this be back ported to 2.x at the earliest opportunity? The same issue occurs there. |
Hey @radex, generally we plan to drop support for v2 but I think we can make an exception for this issue. Let me know if you need any other bugfixes. |
Currently, Reanimated crashes on some configurations due to an assert in `SingleInstanceChecker` which was meant to help us detect retain cycles and thus memory leaks in the library at an early stage but actually causes lots of troubles in client apps. This PR changes the behavior of `SingleInstanceChecker` so that it always prints the warning to the console but calls the failing `assert` only in Reanimated example apps (i.e. Example or FabricExample). This PR wraps the failing assert inside a preprocessor conditional based on the flags passed from `build.gradle` via `CMakeLists.txt` (Android) or `RNReanimated.podspec` (iOS). Additionally, it introduces `__android_log_print` instead of `std::cerr` on Android so that the warning is also visible in logcat. Android: <img width="1493" alt="android" src="https://user-images.githubusercontent.com/20516055/229063204-36af61a7-1ace-4a64-bf43-4cb1caceb44f.png"> iOS: <img width="898" alt="ios" src="https://user-images.githubusercontent.com/20516055/229063251-d1b66d79-58ff-4077-a88c-f8d368d74377.png"> 1. Check if `IS_REANIMATED_EXAMPLE_APP` preprocessor definition is properly injected on both platforms: - Android: change implementation of `isReanimatedExampleApp` in build.gradle to always return `true` - iOS: assign `true` to `result[:is_reanimated_example_app]` in `reanimated_utils.rb` - add the following snippet in `SingleInstanceChecker.h`: ```cpp ``` 2. Check if the example apps crash on reload: - modify the condition in `SingleInstanceChecker.h` so that it always fails - the app should crash on launch 3. Check if the client apps don't crash on reload but show a warning: - modify the condition in `SingleInstanceChecker.h` so that it always fails - build the package locally - create a new RN app and install Reanimated - the app shouldn't crash on launch - there should be a warning in Android Logcat / iOS app output console
@tomekzaw Thanks for the info, this is the only major issue that I've been experiencing right now on v2. (Of course I intend to migrate to v3 eventually, but rewriting many complex REA sequences written with v1 API is going to be painful.) |
@tomekzaw Thanks! |
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [react-native-reanimated](https://togithub.com/software-mansion/react-native-reanimated) | [`~2.14.4` -> `~2.16.0`](https://renovatebot.com/diffs/npm/react-native-reanimated/2.14.4/2.16.0) | [![age](https://badges.renovateapi.com/packages/npm/react-native-reanimated/2.16.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/react-native-reanimated/2.16.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/react-native-reanimated/2.16.0/compatibility-slim/2.14.4)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/react-native-reanimated/2.16.0/confidence-slim/2.14.4)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>software-mansion/react-native-reanimated</summary> ### [`v2.16.0`](https://togithub.com/software-mansion/react-native-reanimated/releases/tag/2.16.0) [Compare Source](https://togithub.com/software-mansion/react-native-reanimated/compare/2.15.0...2.16.0) #### What's Changed - Call assert in SingleInstanceChecker only in example apps by [@​tomekzaw](https://togithub.com/tomekzaw) in [https://github.com/software-mansion/react-native-reanimated/pull/4316](https://togithub.com/software-mansion/react-native-reanimated/pull/4316) - Release 2.16.0 by [@​tomekzaw](https://togithub.com/tomekzaw) in [https://github.com/software-mansion/react-native-reanimated/pull/4389](https://togithub.com/software-mansion/react-native-reanimated/pull/4389) **Full Changelog**: software-mansion/react-native-reanimated@2.15.0...2.16.0 ### [`v2.15.0`](https://togithub.com/software-mansion/react-native-reanimated/releases/tag/2.15.0) [Compare Source](https://togithub.com/software-mansion/react-native-reanimated/compare/2.14.4...2.15.0) #### What's Changed - Fix issue causing ANR when performing sync layout updates, for Reanimated 2 in [https://github.com/software-mansion/react-native-reanimated/pull/4283](https://togithub.com/software-mansion/react-native-reanimated/pull/4283) - Add web support without a Babel/SWC Plugin, for Reanimated 2 in [https://github.com/software-mansion/react-native-reanimated/pull/4285](https://togithub.com/software-mansion/react-native-reanimated/pull/4285) **Full Changelog**: software-mansion/react-native-reanimated@2.14.4...2.15.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/dooboolab-community/dooboo-ui). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS41Ny4wIiwidXBkYXRlZEluVmVyIjoiMzUuNTcuMCJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Hey @radex, I've made a stupid mistake when publishing 2.16.0 (it doesn't contain |
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [react-native-reanimated](https://togithub.com/software-mansion/react-native-reanimated) | [`~2.14.4` -> `~2.17.0`](https://renovatebot.com/diffs/npm/react-native-reanimated/2.14.4/2.17.0) | [![age](https://badges.renovateapi.com/packages/npm/react-native-reanimated/2.17.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/react-native-reanimated/2.17.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/react-native-reanimated/2.17.0/compatibility-slim/2.14.4)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/react-native-reanimated/2.17.0/confidence-slim/2.14.4)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>software-mansion/react-native-reanimated</summary> ### [`v2.17.0`](https://togithub.com/software-mansion/react-native-reanimated/releases/tag/2.17.0) [Compare Source](https://togithub.com/software-mansion/react-native-reanimated/compare/2.16.0...2.17.0) #### What's Changed - Release 2.17.0 by [@​tomekzaw](https://togithub.com/tomekzaw) in [https://github.com/software-mansion/react-native-reanimated/pull/4395](https://togithub.com/software-mansion/react-native-reanimated/pull/4395) **Full Changelog**: software-mansion/react-native-reanimated@2.16.0...2.17.0 ### [`v2.16.0`](https://togithub.com/software-mansion/react-native-reanimated/releases/tag/2.16.0) [Compare Source](https://togithub.com/software-mansion/react-native-reanimated/compare/2.15.0...2.16.0) #### What's Changed - Call assert in SingleInstanceChecker only in example apps by [@​tomekzaw](https://togithub.com/tomekzaw) in [https://github.com/software-mansion/react-native-reanimated/pull/4316](https://togithub.com/software-mansion/react-native-reanimated/pull/4316) - Release 2.16.0 by [@​tomekzaw](https://togithub.com/tomekzaw) in [https://github.com/software-mansion/react-native-reanimated/pull/4389](https://togithub.com/software-mansion/react-native-reanimated/pull/4389) **Full Changelog**: software-mansion/react-native-reanimated@2.15.0...2.16.0 ### [`v2.15.0`](https://togithub.com/software-mansion/react-native-reanimated/releases/tag/2.15.0) [Compare Source](https://togithub.com/software-mansion/react-native-reanimated/compare/2.14.4...2.15.0) #### What's Changed - Fix issue causing ANR when performing sync layout updates, for Reanimated 2 in [https://github.com/software-mansion/react-native-reanimated/pull/4283](https://togithub.com/software-mansion/react-native-reanimated/pull/4283) - Add web support without a Babel/SWC Plugin, for Reanimated 2 in [https://github.com/software-mansion/react-native-reanimated/pull/4285](https://togithub.com/software-mansion/react-native-reanimated/pull/4285) **Full Changelog**: software-mansion/react-native-reanimated@2.14.4...2.15.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/dooboolab-community/dooboo-ui). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS41OC4wIiwidXBkYXRlZEluVmVyIjoiMzUuNTguMCJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…-mansion#4316) ## Why Currently, Reanimated crashes on some configurations due to an assert in `SingleInstanceChecker` which was meant to help us detect retain cycles and thus memory leaks in the library at an early stage but actually causes lots of troubles in client apps. This PR changes the behavior of `SingleInstanceChecker` so that it always prints the warning to the console but calls the failing `assert` only in Reanimated example apps (i.e. Example or FabricExample). ## How This PR wraps the failing assert inside a preprocessor conditional based on the flags passed from `build.gradle` via `CMakeLists.txt` (Android) or `RNReanimated.podspec` (iOS). Additionally, it introduces `__android_log_print` instead of `std::cerr` on Android so that the warning is also visible in logcat. Android: <img width="1493" alt="android" src="https://user-images.githubusercontent.com/20516055/229063204-36af61a7-1ace-4a64-bf43-4cb1caceb44f.png"> iOS: <img width="898" alt="ios" src="https://user-images.githubusercontent.com/20516055/229063251-d1b66d79-58ff-4077-a88c-f8d368d74377.png"> ## Test plan 1. Check if `IS_REANIMATED_EXAMPLE_APP` preprocessor definition is properly injected on both platforms: - Android: change implementation of `isReanimatedExampleApp` in build.gradle to always return `true` - iOS: assign `true` to `result[:is_reanimated_example_app]` in `reanimated_utils.rb` - add the following snippet in `SingleInstanceChecker.h`: ```cpp #ifdef IS_REANIMATED_EXAMPLE_APP #error "it works" #endif ``` 2. Check if the example apps crash on reload: - modify the condition in `SingleInstanceChecker.h` so that it always fails - the app should crash on launch 3. Check if the client apps don't crash on reload but show a warning: - modify the condition in `SingleInstanceChecker.h` so that it always fails - build the package locally - create a new RN app and install Reanimated - the app shouldn't crash on launch - there should be a warning in Android Logcat / iOS app output console
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [react-native-reanimated](https://togithub.com/software-mansion/react-native-reanimated) | [`~2.14.4` -> `~2.17.0`](https://renovatebot.com/diffs/npm/react-native-reanimated/2.14.4/2.17.0) | [![age](https://badges.renovateapi.com/packages/npm/react-native-reanimated/2.17.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/react-native-reanimated/2.17.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/react-native-reanimated/2.17.0/compatibility-slim/2.14.4)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/react-native-reanimated/2.17.0/confidence-slim/2.14.4)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>software-mansion/react-native-reanimated</summary> ### [`v2.17.0`](https://togithub.com/software-mansion/react-native-reanimated/releases/tag/2.17.0) [Compare Source](https://togithub.com/software-mansion/react-native-reanimated/compare/2.16.0...2.17.0) #### What's Changed - Release 2.17.0 by [@​tomekzaw](https://togithub.com/tomekzaw) in [https://github.com/software-mansion/react-native-reanimated/pull/4395](https://togithub.com/software-mansion/react-native-reanimated/pull/4395) **Full Changelog**: software-mansion/react-native-reanimated@2.16.0...2.17.0 ### [`v2.16.0`](https://togithub.com/software-mansion/react-native-reanimated/releases/tag/2.16.0) [Compare Source](https://togithub.com/software-mansion/react-native-reanimated/compare/2.15.0...2.16.0) #### What's Changed - Call assert in SingleInstanceChecker only in example apps by [@​tomekzaw](https://togithub.com/tomekzaw) in [https://github.com/software-mansion/react-native-reanimated/pull/4316](https://togithub.com/software-mansion/react-native-reanimated/pull/4316) - Release 2.16.0 by [@​tomekzaw](https://togithub.com/tomekzaw) in [https://github.com/software-mansion/react-native-reanimated/pull/4389](https://togithub.com/software-mansion/react-native-reanimated/pull/4389) **Full Changelog**: software-mansion/react-native-reanimated@2.15.0...2.16.0 ### [`v2.15.0`](https://togithub.com/software-mansion/react-native-reanimated/releases/tag/2.15.0) [Compare Source](https://togithub.com/software-mansion/react-native-reanimated/compare/2.14.4...2.15.0) #### What's Changed - Fix issue causing ANR when performing sync layout updates, for Reanimated 2 in [https://github.com/software-mansion/react-native-reanimated/pull/4283](https://togithub.com/software-mansion/react-native-reanimated/pull/4283) - Add web support without a Babel/SWC Plugin, for Reanimated 2 in [https://github.com/software-mansion/react-native-reanimated/pull/4285](https://togithub.com/software-mansion/react-native-reanimated/pull/4285) **Full Changelog**: software-mansion/react-native-reanimated@2.14.4...2.15.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/dooboolab-community/dooboo-ui). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMTcuMyIsInVwZGF0ZWRJblZlciI6IjM1LjEzMS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [react-native-reanimated](https://togithub.com/software-mansion/react-native-reanimated) | [`~2.14.4` -> `~2.17.0`](https://renovatebot.com/diffs/npm/react-native-reanimated/2.14.4/2.17.0) | [![age](https://badges.renovateapi.com/packages/npm/react-native-reanimated/2.17.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/react-native-reanimated/2.17.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/react-native-reanimated/2.17.0/compatibility-slim/2.14.4)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/react-native-reanimated/2.17.0/confidence-slim/2.14.4)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>software-mansion/react-native-reanimated (react-native-reanimated)</summary> ### [`v2.17.0`](https://togithub.com/software-mansion/react-native-reanimated/releases/tag/2.17.0) [Compare Source](https://togithub.com/software-mansion/react-native-reanimated/compare/2.16.0...2.17.0) #### What's Changed - Release 2.17.0 by [@​tomekzaw](https://togithub.com/tomekzaw) in [https://github.com/software-mansion/react-native-reanimated/pull/4395](https://togithub.com/software-mansion/react-native-reanimated/pull/4395) **Full Changelog**: software-mansion/react-native-reanimated@2.16.0...2.17.0 ### [`v2.16.0`](https://togithub.com/software-mansion/react-native-reanimated/releases/tag/2.16.0) [Compare Source](https://togithub.com/software-mansion/react-native-reanimated/compare/2.15.0...2.16.0) #### What's Changed - Call assert in SingleInstanceChecker only in example apps by [@​tomekzaw](https://togithub.com/tomekzaw) in [https://github.com/software-mansion/react-native-reanimated/pull/4316](https://togithub.com/software-mansion/react-native-reanimated/pull/4316) - Release 2.16.0 by [@​tomekzaw](https://togithub.com/tomekzaw) in [https://github.com/software-mansion/react-native-reanimated/pull/4389](https://togithub.com/software-mansion/react-native-reanimated/pull/4389) **Full Changelog**: software-mansion/react-native-reanimated@2.15.0...2.16.0 ### [`v2.15.0`](https://togithub.com/software-mansion/react-native-reanimated/releases/tag/2.15.0) [Compare Source](https://togithub.com/software-mansion/react-native-reanimated/compare/2.14.4...2.15.0) #### What's Changed - Fix issue causing ANR when performing sync layout updates, for Reanimated 2 in [https://github.com/software-mansion/react-native-reanimated/pull/4283](https://togithub.com/software-mansion/react-native-reanimated/pull/4283) - Add web support without a Babel/SWC Plugin, for Reanimated 2 in [https://github.com/software-mansion/react-native-reanimated/pull/4285](https://togithub.com/software-mansion/react-native-reanimated/pull/4285) **Full Changelog**: software-mansion/react-native-reanimated@2.14.4...2.15.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/dooboolab-community/dooboo-ui). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xNDEuMyIsInVwZGF0ZWRJblZlciI6IjM1LjE0NC4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Why
Currently, Reanimated crashes on some configurations due to an assert in
SingleInstanceChecker
which was meant to help us detect retain cycles and thus memory leaks in the library at an early stage but actually causes lots of troubles in client apps. This PR changes the behavior ofSingleInstanceChecker
so that it always prints the warning to the console but calls the failingassert
only in Reanimated example apps (i.e. Example or FabricExample).How
This PR wraps the failing assert inside a preprocessor conditional based on the flags passed from
build.gradle
viaCMakeLists.txt
(Android) orRNReanimated.podspec
(iOS). Additionally, it introduces__android_log_print
instead ofstd::cerr
on Android so that the warning is also visible in logcat.Android:
iOS:
Test plan
IS_REANIMATED_EXAMPLE_APP
preprocessor definition is properly injected on both platforms:isReanimatedExampleApp
in build.gradle to always returntrue
true
toresult[:is_reanimated_example_app]
inreanimated_utils.rb
SingleInstanceChecker.h
:SingleInstanceChecker.h
so that it always failsSingleInstanceChecker.h
so that it always fails