Fix issue causing ANR when performing sync layout updates #4239

kmagiera · 2023-03-17T14:00:47Z

Summary

This PR addresses the problem from #3879 on iOS where application would fall into a deadlock when performing synchronous layout updates. The reason for the issue was that sync layout updates requires UI thread to lock and wait for layout thread to perform some tasks. On the other hand, layout thread sometimes schedules UI tasks to be performed synchronously (with dispatch_sync mechanism).

The solution this PR implements is to add a timeout on waiting and fallback to updating layout props asynchronously. This feels like a right approach as the cases when dispatch_sync is used are relatively rare and the consequences of updating layout asynchronously aren't serious (there will be a one frame delay for that one update).

This solution is based on #3082 but on top of just adding the timeout we also need to make sure that the executed operations are properly mounted. The problem with #3082 was that when the UI thread times out we still use runSyncUIUpdatesWithObserver call on the layout thread which in turn results in the mounting block being stored and expected to be run on the locked UI thread afterwards. If the thread times out we may hit an issue when the mounting block is set but never executed which would have serious implications.

Test plan

Tested this with repro scenario provided in #3946 that relies on @react-native-community/datetimepicker library

tomekzaw · 2023-03-18T20:19:51Z

ios/REANodesManager.mm

        dispatch_semaphore_signal(semaphore);
      }
      // In case canUpdateSynchronously=true we still have to send uiManagerWillPerformMounting event
      // to observers because some components (e.g. TextInput) update their UIViews only on that event.
      [strongSelf.uiManager setNeedsLayout];
    });
    if (trySynchronously) {
-      dispatch_semaphore_wait(semaphore, DISPATCH_TIME_FOREVER);
+      // The 16ms timeout here aims to match the frame duration. It may make sense to read that parameter


What about 120 fps mode?

Was trying to keep this change minimal as may want to port it back to Rea 2. We could pass expected frame duration here but we don't always have it available (e.g. we have it when we run operations from CADisplayLink callback but when handling events display link instance could be paused). In my opinion the consequences of this timeout being longer than one frame on 120fps devices isn't very serious. This deadlock happens very randomly and not that often. If the timeout takes two frames we will just drop one extra frame on such device.

piaskowyk

Generally looks good to me but I have two question:

If React attempts to mount a large number of components simultaneously and the lock expires, will this approach still works?
Did you test it on Fabric also?

piaskowyk · 2023-03-20T14:22:54Z

ios/REANodesManager.mm

@@ -96,6 +96,8 @@ @implementation REANodesManager {
  BOOL _tryRunBatchUpdatesSynchronously;
  REAEventHandler _eventHandler;
  volatile void (^_mounting)(void);
+  NSObject *_syncLayoutUpdatesWaitLock;
+  volatile BOOL _syncLayoutUpdatesWaitTimedOut;


We should set initial value for _syncLayoutUpdatesWaitTimedOut - just for sure.

Maybe the usage of atomic boolean instead of volatile BOOL + lock would be better approach here.

we always reset that bool prior to the place where we read from it

this won't work as we want to synchronize more changes, specifically setting _mounting property also happens inside of the synchronized block and has to be synchronized for this to work. Using atomic bool won't suffice

@WoLewicki

This PR adds a cool emoji waterfall example. https://user-images.githubusercontent.com/20516055/225790891-8be3be80-55df-49f7-af81-2365d3e625db.mp4 Just see if it works. docs: add app.js banner (software-mansion#4183) This PR adds banner announcing app.js conf. The banner is easily dismissable and will disappear automatically after the conference. ![image](https://user-images.githubusercontent.com/39658211/223506267-de56f51f-e517-477c-ae31-5aaad293a629.png) <img width="721" alt="image" src="https://user-images.githubusercontent.com/39658211/223506461-b92c6a1f-1794-4a6f-ae54-01436bdcd729.png"> ![image](https://user-images.githubusercontent.com/39658211/223508037-c192d3f2-039f-496c-8e59-9063546723f9.png) ![image](https://user-images.githubusercontent.com/39658211/223508080-f678c7df-1c84-4834-be74-a5f123eadb7a.png) fix `Error` Variable scope conflicts, and remove duplicate `Date` (software-mansion#4046)  The babel plugin transformed the code and added some helper code, it uses `new Error()` to get the call stack. this works in general unless you have a variable called `Error` in the local scope, in this case, It references the wrong Object. this PR uses `global.Error` to avoid this. `'Date'` is duplicated in this `global` array, and I removed the second one. ```jsx import { useState } from 'react'; import Animated, { useAnimatedStyle, useSharedValue } from 'react-native-reanimated'; import Error from './components/Error'; // imported an arrow function component named 'Error' const App = () => { const [hasError, setHasError] = useState(false); const offset = useSharedValue(0); // `new Error()` not working here, because `Error` reference to the function component, it's a arrow function, not a function or class const animatedStyles = useAnimatedStyle(() => ({ transform: [{ translateX: offset.value * 255 }] }), []); if (hasError) return <Error />; return <Animated.View style={animatedStyles} />; }; ``` --------- Rewrite plugin to TypeScript (software-mansion#4096) Rewriting plugin to typescript. Plugin will compile when doing either `yarn` or `yarn plugin`, hence it will be available in npm tarball. I think the best way to review the changes is to compare the compiled 'index.js' file with the old one (having in mind compilation options in 'tsconfig.json'), then remove it in the follow-up commit. Generate bundles for `Example` and `FabricApp` for `main`. Generate bundles for current head of this branch with freshly compiled `index.js`. Compare the bundles. There should be no differences as long as they were built in the same directory. --------- Actually apply changes from software-mansion#4046 (software-mansion#4190) This PR fixes failing TypeScript CI on main branch after merging software-mansion#4096. In particular, this PR actually applies the changes from software-mansion#4046 (which most likely were skipped due to improper git conflict resolution), including `global.Error` fix and removing duplicate `Date` entry, as well as updates one snapshot which for some reason was left out. Just see if the CI is green. Add .prettierignore file to stop CI fails on plugin compiled files commits (software-mansion#4193) Don't allow prettier to automatically prettify compiled files in plugin directory. Check if CI passes. [V3 bugfix] Change private methods to public ones (software-mansion#4205)   This issue: software-mansion#4135 describes an error happening on old Android versions. The error message is > 🚫 ${\color{red}\text{Error}}$ > Exception in HostFunction: java.lang.NoSuchMethodError: no non-static method "Lcom/swmansion/reanimated/NativeProxy;.registerEventHandler(Lcom/swmansion/reanimated/nativeProxy/EventHandler;)V", js engine: hermes It can be simply fixed by changing some function from `private` to `public` ones as suggested [here](software-mansion#4135 (comment)). Fixes software-mansion#4135  Tested on android simulator Bump webpack from 5.74.0 to 5.76.1 in /docs (software-mansion#4224) Bumps [webpack](https://github.com/webpack/webpack) from 5.74.0 to 5.76.1. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/webpack/webpack/releases">webpack's releases</a>.</em></p> <blockquote> <h2>v5.76.1</h2> <h2>Fixed</h2> <ul> <li>Added <code>assert/strict</code> built-in to <code>NodeTargetPlugin</code></li> </ul> <h2>Revert</h2> <ul> <li>Improve performance of <code>hashRegExp</code> lookup by <a href="https://github.com/ryanwilsonperkin"><code>@ryanwilsonperkin</code></a> in <a href="https://github.com/webpack/webpack/pull/16759">webpack/webpack#16759</a></li> </ul> <h2>v5.76.0</h2> <h2>Bugfixes</h2> <ul> <li>Avoid cross-realm object access by <a href="https://github.com/Jack-Works"><code>@Jack-Works</code></a> in <a href="https://github.com/webpack/webpack/pull/16500">webpack/webpack#16500</a></li> <li>Improve hash performance via conditional initialization by <a href="https://github.com/lvivski"><code>@lvivski</code></a> in <a href="https://github.com/webpack/webpack/pull/16491">webpack/webpack#16491</a></li> <li>Serialize <code>generatedCode</code> info to fix bug in asset module cache restoration by <a href="https://github.com/ryanwilsonperkin"><code>@ryanwilsonperkin</code></a> in <a href="https://github.com/webpack/webpack/pull/16703">webpack/webpack#16703</a></li> <li>Improve performance of <code>hashRegExp</code> lookup by <a href="https://github.com/ryanwilsonperkin"><code>@ryanwilsonperkin</code></a> in <a href="https://github.com/webpack/webpack/pull/16759">webpack/webpack#16759</a></li> </ul> <h2>Features</h2> <ul> <li>add <code>target</code> to <code>LoaderContext</code> type by <a href="https://github.com/askoufis"><code>@askoufis</code></a> in <a href="https://github.com/webpack/webpack/pull/16781">webpack/webpack#16781</a></li> </ul> <h2>Security</h2> <ul> <li><a href="https://github.com/advisories/GHSA-3rfm-jhwj-7488">CVE-2022-37603</a> fixed by <a href="https://github.com/akhilgkrishnan"><code>@akhilgkrishnan</code></a> in <a href="https://github.com/webpack/webpack/pull/16446">webpack/webpack#16446</a></li> </ul> <h2>Repo Changes</h2> <ul> <li>Fix HTML5 logo in README by <a href="https://github.com/jakebailey"><code>@jakebailey</code></a> in <a href="https://github.com/webpack/webpack/pull/16614">webpack/webpack#16614</a></li> <li>Replace TypeScript logo in README by <a href="https://github.com/jakebailey"><code>@jakebailey</code></a> in <a href="https://github.com/webpack/webpack/pull/16613">webpack/webpack#16613</a></li> <li>Update actions/cache dependencies by <a href="https://github.com/piwysocki"><code>@piwysocki</code></a> in <a href="https://github.com/webpack/webpack/pull/16493">webpack/webpack#16493</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a href="https://github.com/Jack-Works"><code>@Jack-Works</code></a> made their first contribution in <a href="https://github.com/webpack/webpack/pull/16500">webpack/webpack#16500</a></li> <li><a href="https://github.com/lvivski"><code>@lvivski</code></a> made their first contribution in <a href="https://github.com/webpack/webpack/pull/16491">webpack/webpack#16491</a></li> <li><a href="https://github.com/jakebailey"><code>@jakebailey</code></a> made their first contribution in <a href="https://github.com/webpack/webpack/pull/16614">webpack/webpack#16614</a></li> <li><a href="https://github.com/akhilgkrishnan"><code>@akhilgkrishnan</code></a> made their first contribution in <a href="https://github.com/webpack/webpack/pull/16446">webpack/webpack#16446</a></li> <li><a href="https://github.com/ryanwilsonperkin"><code>@ryanwilsonperkin</code></a> made their first contribution in <a href="https://github.com/webpack/webpack/pull/16703">webpack/webpack#16703</a></li> <li><a href="https://github.com/piwysocki"><code>@piwysocki</code></a> made their first contribution in <a href="https://github.com/webpack/webpack/pull/16493">webpack/webpack#16493</a></li> <li><a href="https://github.com/askoufis"><code>@askoufis</code></a> made their first contribution in <a href="https://github.com/webpack/webpack/pull/16781">webpack/webpack#16781</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/webpack/webpack/compare/v5.75.0...v5.76.0">https://github.com/webpack/webpack/compare/v5.75.0...v5.76.0</a></p> <h2>v5.75.0</h2> <h1>Bugfixes</h1> <ul> <li><code>experiments.*</code> normalize to <code>false</code> when opt-out</li> <li>avoid <code>NaN%</code></li> <li>show the correct error when using a conflicting chunk name in code</li> <li>HMR code tests existance of <code>window</code> before trying to access it</li> <li>fix <code>eval-nosources-*</code> actually exclude sources</li> <li>fix race condition where no module is returned from processing module</li> <li>fix position of standalong semicolon in runtime code</li> </ul> <h1>Features</h1> <ul> <li>add support for <code>@import</code> to extenal CSS when using experimental CSS in node</li> </ul>  </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/webpack/webpack/commit/21be52b681c477f8ebc41c1b0e7a7a8ac4fa7008"><code>21be52b</code></a> Merge pull request <a href="https://github.com/webpack/webpack/issues/16804">#16804</a> from webpack/chore-patch-release</li> <li><a href="https://github.com/webpack/webpack/commit/1cce945dd6c3576d37d3940a0233fd087ce3f6ff"><code>1cce945</code></a> chore(release): 5.76.1</li> <li><a href="https://github.com/webpack/webpack/commit/e76ad9e724410f10209caa2ba86875ca8cf5ed61"><code>e76ad9e</code></a> Merge pull request <a href="https://github.com/webpack/webpack/issues/16803">#16803</a> from ryanwilsonperkin/revert-16759-real-content-has...</li> <li><a href="https://github.com/webpack/webpack/commit/52b1b0e4ada7c11e7f1b4f3d69b50684938c684e"><code>52b1b0e</code></a> Revert "Improve performance of hashRegExp lookup"</li> <li><a href="https://github.com/webpack/webpack/commit/c989143379d344543e4161fec60f3a21beb9e3ce"><code>c989143</code></a> Merge pull request <a href="https://github.com/webpack/webpack/issues/16766">#16766</a> from piranna/patch-1</li> <li><a href="https://github.com/webpack/webpack/commit/710eaf4ddaea505e040a24beeb45a769f9e3761b"><code>710eaf4</code></a> Merge pull request <a href="https://github.com/webpack/webpack/issues/16789">#16789</a> from dmichon-msft/contenthash-hashsalt</li> <li><a href="https://github.com/webpack/webpack/commit/5d6446822aff579a5d3d9503ec2a16437d2f71d1"><code>5d64468</code></a> Merge pull request <a href="https://github.com/webpack/webpack/issues/16792">#16792</a> from webpack/update-version</li> <li><a href="https://github.com/webpack/webpack/commit/67af5ec1f05fb7cf06be6acf27353aef105ddcbc"><code>67af5ec</code></a> chore(release): 5.76.0</li> <li><a href="https://github.com/webpack/webpack/commit/97b1718720c33f1b17302a74c5284b01e02ec001"><code>97b1718</code></a> Merge pull request <a href="https://github.com/webpack/webpack/issues/16781">#16781</a> from askoufis/loader-context-target-type</li> <li><a href="https://github.com/webpack/webpack/commit/b84efe6224b276bf72e4c5e2f4e76acddfaeef07"><code>b84efe6</code></a> Merge pull request <a href="https://github.com/webpack/webpack/issues/16759">#16759</a> from ryanwilsonperkin/real-content-hash-regex-perf</li> <li>Additional commits viewable in <a href="https://github.com/webpack/webpack/compare/v5.74.0...v5.76.1">compare view</a></li> </ul> </details> <details> <summary>Maintainer changes</summary> <p>This version was pushed to npm by <a href="https://www.npmjs.com/~evilebottnawi">evilebottnawi</a>, a new releaser for webpack since your current version.</p> </details> <br /> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=webpack&package-manager=npm_and_yarn&previous-version=5.74.0&new-version=5.76.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) - `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language - `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language - `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language - `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/software-mansion/react-native-reanimated/network/alerts). </details> Signed-off-by: dependabot[bot] <support@github.com> Add information about RN architecture limitation (software-mansion#4220) Shared Elements Transition documentation page update. Added information about RN architecture limitation. Fixes software-mansion#4167 Bump @sideway/formula from 3.0.0 to 3.0.1 (software-mansion#4230) Bumps [@sideway/formula](https://github.com/sideway/formula) from 3.0.0 to 3.0.1. <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/hapijs/formula/commit/5b44c1bffc38135616fb91d5ad46eaf64f03d23b"><code>5b44c1b</code></a> 3.0.1</li> <li><a href="https://github.com/hapijs/formula/commit/9fbc20a02d75ae809c37a610a57802cd1b41b3fe"><code>9fbc20a</code></a> chore: better number regex</li> <li><a href="https://github.com/hapijs/formula/commit/41ae98e0421913b100886adb0107a25d552d9e1a"><code>41ae98e</code></a> Cleanup</li> <li><a href="https://github.com/hapijs/formula/commit/c59f35ec401e18cead10e0cedfb44291517610b1"><code>c59f35e</code></a> Move to Sideway</li> <li>See full diff in <a href="https://github.com/sideway/formula/compare/v3.0.0...v3.0.1">compare view</a></li> </ul> </details> <details> <summary>Maintainer changes</summary> <p>This version was pushed to npm by <a href="https://www.npmjs.com/~marsup">marsup</a>, a new releaser for <code>@sideway/formula</code> since your current version.</p> </details> <br /> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=@sideway/formula&package-manager=npm_and_yarn&previous-version=3.0.0&new-version=3.0.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) - `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language - `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language - `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language - `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/software-mansion/react-native-reanimated/network/alerts). </details> --------- Signed-off-by: dependabot[bot] <support@github.com> Bump activesupport from 6.1.7.1 to 6.1.7.3 in /TVOSExample (software-mansion#4234) Bumps [activesupport](https://github.com/rails/rails) from 6.1.7.1 to 6.1.7.3. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/rails/rails/releases">activesupport's releases</a>.</em></p> <blockquote> <h2>v6.1.7.3</h2> <h2>Active Support</h2> <ul> <li> <p>Implement SafeBuffer#bytesplice</p> <p>[CVE-2023-28120]</p> </li> </ul> <h2>Active Model</h2> <ul> <li>No changes.</li> </ul> <h2>Active Record</h2> <ul> <li>No changes.</li> </ul> <h2>Action View</h2> <ul> <li> <p>Ignore certain data-* attributes in rails-ujs when element is contenteditable</p> <p>[CVE-2023-23913]</p> </li> </ul> <h2>Action Pack</h2> <ul> <li>No changes.</li> </ul> <h2>Active Job</h2> <ul> <li>No changes.</li> </ul> <h2>Action Mailer</h2> <ul> <li>No changes.</li> </ul> <h2>Action Cable</h2>  </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/rails/rails/commit/f09dc7c4c2e8b9375345d443c230cb8d78ad6a18"><code>f09dc7c</code></a> Preparing for 6.1.7.3 release</li> <li><a href="https://github.com/rails/rails/commit/7167e535201afc1d1864dd9eb862f177e309ade3"><code>7167e53</code></a> Prepare version 6.1.7.3</li> <li><a href="https://github.com/rails/rails/commit/3cf23c3f891e2e81c977ea4ab83b62bc2a444b70"><code>3cf23c3</code></a> Implement SafeBuffer#bytesplice</li> <li><a href="https://github.com/rails/rails/commit/3e0c1a528494c352c125962c1f96b369e1d5ac34"><code>3e0c1a5</code></a> Version 6.1.7.2</li> <li>See full diff in <a href="https://github.com/rails/rails/compare/v6.1.7.1...v6.1.7.3">compare view</a></li> </ul> </details> <br /> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=activesupport&package-manager=bundler&previous-version=6.1.7.1&new-version=6.1.7.3)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) - `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language - `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language - `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language - `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/software-mansion/react-native-reanimated/network/alerts). </details> Signed-off-by: dependabot[bot] <support@github.com> Fix a typo (software-mansion#4246)    chore: set library namespace in build script (software-mansion#4208)  Since AGP 8.0 project namespace [**must** be set inside build script](https://issuetracker.google.com/issues/172361895). It is also the recommended way in [Android docs](https://developer.android.com/studio/build/configure-app-module?authuser=1#set-namespace). It is also recommended, that the package name definition is removed from Android manifest file, but I've had some problems in `react-native-screens` with RN CLI crashing (as it was expecting the [`package` field](https://github.com/software-mansion/react-native-screens/blob/2a30e22bf1bd432fdd5e2f4d44e42bc3efd44474/android/src/main/AndroidManifest.xml#L3) to exist) & according to [this](https://issuetracker.google.com/issues/200682321?pli=1) conversation it'll still be legal to leave package name definition in Android manifest file as long as it exactly matches the value in build script. See: software-mansion/react-native-screens#1717 CI Fix corrupted yarn.locks (software-mansion#4249) Our `yarn.lock` files are corrupted and somehow they were not updating properly when using `yarn`. That caused adding multiple versions of some libraries into `node_modules` and TypeScript was going 🤪. I removed all the `yarn.lock` files and recreated them with `yarn` and then had to fix all the issues that were coming with that. @WoLewicki stated that `FabricExample/patches/react-native-svg+13.7.0.patch` is not necessary anymore. Bumped version in the name of the other patch file since we didn't forbid a bump anyway. ```diff --- a/__tests__/__snapshots__/plugin.test.js.snap +++ b/__tests__/__snapshots__/plugin.test.js.snap @@ -400,7 +400,7 @@ var foo = function () { }, { b: 2, c: 3 - }, {}, bar); + }, bar); }; _f._closure = {}; _f.__initData = _worklet_792186851025_init_data; ``` This was actually an overlooked bug and somehow got fixed with these changes (my guess would be that newer versions of plugins for babel that got included fixed it). ```diff --- a/babel.config.js +++ b/babel.config.js @@ -14,5 +14,11 @@ module.exports = { 'module:metro-react-native-babel-preset', ], plugins: [ '@babel/plugin-proposal-class-properties', ['./plugin', { disableInlineStylesWarning: true }], + [ + '@babel/plugin-transform-react-jsx', + { + runtime: 'classic', + }, + ], ], }; ``` This isn't necessary but I would recommend it for now. The reason for this change is the fact that in newer versions of plugins that are included in `module:metro-react-native-babel-preset` [jsx-runtime is set as default](https://legacy.reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html) and since we don't include old versions anymore we have to restore this behaviour. If we don't include this change babel will for eg. do: ```diff @@ -1,7 +1,8 @@ + var _jsxRuntime = require("react/jsx-runtime"); function App() { - return React.createElement(Animated.View, { + return (0, _jsxRuntime.jsx)(Animated.View, { ``` Which I guess is not bad for us since we actually use it if we develop apps that run this preset and use default config. For now though I'd suggest keeping this change. ```diff --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "format:common": "find Common/ -iname *.h -o -iname *.cpp | xargs clang-format -i", "release": "npm login && release-it", "type:check": "yarn tsc --noEmit && cd plugin && yarn type:check && cd ..", - "prepare": "bob build && husky install", + "prepare": "bob build && husky install && yarn plugin", "circular_dependency_check": "yarn madge --extensions js,ts,tsx --circular src lib", "setup": "yarn && cd Example && yarn && cd ios && pod install --verbose && cd ../..", "clean": "rm -rf node_modules && cd Example && rm -rf node_modules && cd ios && pod deintegrate && cd ../..", @@ -91,6 +91,7 @@ "react-native": "*" }, "devDependencies": { + "@types/node": "12.7.12", "@babel/cli": "^7.17.6", "@babel/core": "^7.20.0", "@babel/plugin-proposal-class-properties": "^7.16.7", ``` First change is self-explanatory. Second one is more tricky. With the old version of `yarn.lock` we had `@types/node` with exactly this version( it's 3 years old!) installed when doing `yarn`. Regenerating `yarn.lock` and doing `yarn` would install much newer version and cause a TypeScript issue, because in newer versions of `@types/node` there is an extra file `@types/node/globals.global.d.ts` which causes our usage of `global` to produce a lot of errors. I plan to dive into this issue later on but for now I want to just keep status quo. ```diff --- a/src/reanimated2/animation/styleAnimation.ts +++ b/src/reanimated2/animation/styleAnimation.ts @@ -24,7 +24,11 @@ export function resolvePath<T>( return keys.reduce<NestedObjectValues<T> | undefined>((acc, current) => { if (Array.isArray(acc) && typeof current === 'number') { return acc[current]; - } else if (typeof acc === 'object' && (current as number | string) in acc) { + } else if ( + acc !== null && + typeof acc === 'object' && + (current as number | string) in acc + ) { return (acc as { [key: string]: NestedObjectValues<T> })[ current as number | string ]; ``` null guard (error stemmed from bumping packages through new yarn.lock) ```diff --- a/src/reanimated2/shareables.ts +++ b/src/reanimated2/shareables.ts @@ -125,7 +125,7 @@ export function makeShareableCloneOnUIRecursive<T>(value: T): ShareableRef<T> { let toAdapt: any; if (Array.isArray(value)) { toAdapt = value.map((element) => cloneRecursive(element)); - } else { + } else if (value !== undefined) { toAdapt = {}; for (const [key, element] of Object.entries(value)) { toAdapt[key] = cloneRecursive(element); ``` undefined guard (error stemmed from bumping packages through new yarn.lock) - `yarn jest` - build Example and run it - build FabricExample and run it - build TVOSExample and run it to see that it doesn't work same as on current main - build WebExample and run it to see that it might not work same as on current main Fix issue causing ANR when performing sync layout updates (software-mansion#4239) This PR addresses the problem from software-mansion#3879 on iOS where application would fall into a deadlock when performing synchronous layout updates. The reason for the issue was that sync layout updates requires UI thread to lock and wait for layout thread to perform some tasks. On the other hand, layout thread sometimes schedules UI tasks to be performed synchronously (with `dispatch_sync` mechanism). The solution this PR implements is to add a timeout on waiting and fallback to updating layout props asynchronously. This feels like a right approach as the cases when dispatch_sync is used are relatively rare and the consequences of updating layout asynchronously aren't serious (there will be a one frame delay for that one update). This solution is based on software-mansion#3082 but on top of just adding the timeout we also need to make sure that the executed operations are properly mounted. The problem with software-mansion#3082 was that when the UI thread times out we still use `runSyncUIUpdatesWithObserver` call on the layout thread which in turn results in the mounting block being stored and expected to be run on the locked UI thread afterwards. If the thread times out we may hit an issue when the mounting block is set but never executed which would have serious implications. Tested this with repro scenario provided in software-mansion#3946 that relies on `@react-native-community/datetimepicker` library Fix monorepo worklow (software-mansion#4255) Fixing our red main as `react native run-ios` is having troubles with launching a default (non-specified) simulator. CI is the way. Co-Authored-By: Tomasz Żelawski <40713406+tjzel@users.noreply.github.com> Co-Authored-By: Tomek Zawadzki <tomasz.zawadzki@swmansion.com> Co-Authored-By: Aleksandra Cynk <aleksandracynk@Aleksandras-MacBook-Pro.local> Co-Authored-By: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…ated 2 (#4283) ## Summary The port of #4239 PR for Reanimated 2.

robinvk6 · 2023-03-28T15:53:15Z

When is this fix planned to be released?

cltnschlosser · 2023-03-29T21:09:42Z

Hey @kmagiera coming from #3194 and curious on your thoughts. Do you think these "synchronous" updates actually provide any value at all? My approach there was just to remove them entirely, I've had a yarn patch with some variation of that PR in production for nearly a year now without issues. It seems to me all these "synchronous" updates do is stall the main thread while the update is happening on the UIManager thread, not really anything synchronous about them (just blocking - and potentially deadlocking before this or similar change). Now that you've spent some time with this issue, in your opinion is there any reason to not just do every update async?

kmagiera · 2023-03-31T13:28:33Z

There are two reasons why we added these:

Update consistency: when there are layout+non layout updates performed as a part of animation frame, without sync updates we will see the non-layout props update one frame ahead
Keeping up with events: for layout props updated as a part of event (e.g. scroll or gesture) we want them to be flushed immediately. W/o sync updates we always would get these props lagging after the event stream (particularily noticeable when you try to implement sth like parallax effect with scrollview)

@gavrix

## Summary This PR tries to address the problem with assert that we make in `REANodesManager.mm` when receiving mounting block from the React's UI Manager. The issue was due to a fact that we only hold a single reference for mounting block as well as the timed-out flag, while under certain conditions, the `performOperations` may re-enter before these values are cleaned up correctly. This didn't happen before #4239 because the lock would guarantee that `performIOperation` is never called again before the block scheduled on UIManagerQueue is finished. However in #4239 we changed this behavior to stop potential ANR issues due to locking and this caused this new issue to surface. The change we are making here is that we create a new instance of observer that is specific to a given `performOperation` call. This way every call to `performOperation` shares an instance of observer with UIManagerQueue bloc, which keeps ref to mounting block and timeout flag, hence it is never possible for read/writes of these refs to interfere between subsequent `performOperation` runs. We are now also moving the assert to the new place – to the observer dealloc. We always expect the mounting block to be executed (and cleaned up) and hence if the observer is deallocated with the mounting block set, we treat this as an error. ## Test plan We couldn't manage to reproduce this issue but it was tested courtesy of @gavrix who could reliably reproduce the assert failure on one of the apps he works on.

…ansion#4239) ## Summary This PR addresses the problem from software-mansion#3879 on iOS where application would fall into a deadlock when performing synchronous layout updates. The reason for the issue was that sync layout updates requires UI thread to lock and wait for layout thread to perform some tasks. On the other hand, layout thread sometimes schedules UI tasks to be performed synchronously (with `dispatch_sync` mechanism). The solution this PR implements is to add a timeout on waiting and fallback to updating layout props asynchronously. This feels like a right approach as the cases when dispatch_sync is used are relatively rare and the consequences of updating layout asynchronously aren't serious (there will be a one frame delay for that one update). This solution is based on software-mansion#3082 but on top of just adding the timeout we also need to make sure that the executed operations are properly mounted. The problem with software-mansion#3082 was that when the UI thread times out we still use `runSyncUIUpdatesWithObserver` call on the layout thread which in turn results in the mounting block being stored and expected to be run on the locked UI thread afterwards. If the thread times out we may hit an issue when the mounting block is set but never executed which would have serious implications. ## Test plan Tested this with repro scenario provided in software-mansion#3946 that relies on `@react-native-community/datetimepicker` library

@gavrix

## Summary This PR tries to address the problem with assert that we make in `REANodesManager.mm` when receiving mounting block from the React's UI Manager. The issue was due to a fact that we only hold a single reference for mounting block as well as the timed-out flag, while under certain conditions, the `performOperations` may re-enter before these values are cleaned up correctly. This didn't happen before software-mansion#4239 because the lock would guarantee that `performIOperation` is never called again before the block scheduled on UIManagerQueue is finished. However in software-mansion#4239 we changed this behavior to stop potential ANR issues due to locking and this caused this new issue to surface. The change we are making here is that we create a new instance of observer that is specific to a given `performOperation` call. This way every call to `performOperation` shares an instance of observer with UIManagerQueue bloc, which keeps ref to mounting block and timeout flag, hence it is never possible for read/writes of these refs to interfere between subsequent `performOperation` runs. We are now also moving the assert to the new place – to the observer dealloc. We always expect the mounting block to be executed (and cleaned up) and hence if the observer is deallocated with the mounting block set, we treat this as an error. ## Test plan We couldn't manage to reproduce this issue but it was tested courtesy of @gavrix who could reliably reproduce the assert failure on one of the apps he works on.

Fix issue causing ANR when performing sync layout updates on iOS

d4de467

kmagiera force-pushed the fix-sync-updates-and-on-ios branch from 471ab30 to d4de467 Compare March 17, 2023 23:35

tomekzaw reviewed Mar 18, 2023

View reviewed changes

Add timeout for semaphore wait fort sync layout updates on Android

eaea90d

kmagiera changed the title ~~Fix issue causing ANR when performing sync layout updates on iOS~~ Fix issue causing ANR when performing sync layout updates Mar 20, 2023

piaskowyk reviewed Mar 20, 2023

View reviewed changes

piaskowyk approved these changes Mar 20, 2023

View reviewed changes

Fix lint

8daa5ac

kmagiera added this pull request to the merge queue Mar 21, 2023

kmagiera merged commit b0ee3de into main Mar 21, 2023

kmagiera deleted the fix-sync-updates-and-on-ios branch March 21, 2023 13:16

This was referenced Mar 24, 2023

☂️ Deadlock/ANR in performOperations #3879

Closed

Fix deadlock in NodesManager.java (removes synchronous UI updates) #3440

Closed

piaskowyk mentioned this pull request Mar 27, 2023

Fix issue causing ANR when performing sync layout updates, for Reanimated 2 #4283

Merged

piaskowyk added a commit that referenced this pull request Mar 27, 2023

Fix issue causing ANR when performing sync layout updates, for Reanim…

2905e1b

…ated 2 (#4283) ## Summary The port of #4239 PR for Reanimated 2.

terrysahaidak mentioned this pull request Mar 27, 2023

When you long-press on a message in iOS mobile, the message moves out of the screen frame and is hidden (iOS only) Expensify/App#16356

Closed

54 tasks

This was referenced Mar 29, 2023

Use timeout of 100ms on semaphore in performOperations #3082

Closed

Fix deadlock in REANodesManager (removes synchronous UI updates) #3194

Closed

kmagiera mentioned this pull request Apr 25, 2023

Resolve sync UI updates concurency issue on iOS #4403

Merged

robwalkerco mentioned this pull request May 3, 2023

ANR com.facebook.jni.CppException #4248

Closed

This was referenced May 23, 2023

ANR caused in performOperations #2251

Closed

[IOS] Random app crash on production build #2327

Closed

piaskowyk mentioned this pull request Aug 25, 2023

Deadlock in [REANodesManager onAnimationFrame] #3862

Closed

Latropos mentioned this pull request Sep 22, 2023

ANR : Input dispatching timed out #2812

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issue causing ANR when performing sync layout updates #4239

Fix issue causing ANR when performing sync layout updates #4239

kmagiera commented Mar 17, 2023 •

edited

Loading

tomekzaw Mar 18, 2023

kmagiera Mar 20, 2023

piaskowyk left a comment •

edited

Loading

piaskowyk Mar 20, 2023

kmagiera Mar 21, 2023

robinvk6 commented Mar 28, 2023

cltnschlosser commented Mar 29, 2023 •

edited

Loading

kmagiera commented Mar 31, 2023

Fix issue causing ANR when performing sync layout updates #4239

Fix issue causing ANR when performing sync layout updates #4239

Conversation

kmagiera commented Mar 17, 2023 • edited Loading

Summary

Test plan

tomekzaw Mar 18, 2023

Choose a reason for hiding this comment

kmagiera Mar 20, 2023

Choose a reason for hiding this comment

piaskowyk left a comment • edited Loading

Choose a reason for hiding this comment

piaskowyk Mar 20, 2023

Choose a reason for hiding this comment

kmagiera Mar 21, 2023

Choose a reason for hiding this comment

robinvk6 commented Mar 28, 2023

cltnschlosser commented Mar 29, 2023 • edited Loading

kmagiera commented Mar 31, 2023

kmagiera commented Mar 17, 2023 •

edited

Loading

piaskowyk left a comment •

edited

Loading

cltnschlosser commented Mar 29, 2023 •

edited

Loading