Testing comments #2
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: Pull Request resolved: facebook#51690 This change adds the reactRuntime target to SwiftPM ## Changelog: [Internal] - Add reactRuntime Reviewed By: cortinico Differential Revision: D75638242 fbshipit-source-id: 900a1254e3084a78f41aec5e51f7513724c152ec
Summary: Pull Request resolved: facebook#51694 Changelog: [Internal] D74820232 was missing an `exports` entry for `debugger-shell/package.json`, which is needed for some of our build tooling. Reviewed By: hoxyq Differential Revision: D75679347 fbshipit-source-id: 8deaf55ce354afcff410104948cad1b9a30093b7
Summary: Pull Request resolved: facebook#51697 changelog: [internal] as a general rule of thumb, do not call outside of your class when holding a mutex. It is easy to cause a deadlock because the outside code may end up trying to acquire the mutex down the stack, leading to deadlock. Here, it happens because `startRenderCallbackIfNeeded` may end up calling onRender and in onRender, we try to acquire the mutex again Reviewed By: javache Differential Revision: D75675465 fbshipit-source-id: 46168ee154a54ae5cccaa74728b41f027519db59
…ody of the test (facebook#51696) Summary: Pull Request resolved: facebook#51696 Changelog: [internal] I realized that the Fantom module was being initialized before the execution of the test module itself (as part of the code generated by the runner), which could have problems if the test sets up the global environment that Fantom consumes. For example, if Fantom needs to use the `EventTarget` type from the global scope, it needs to wait until the initialization of the runtime so it can use it safely. This refactors all the code calling into Fantom as part of our infra to always initialize it lazily, so the first thing that runs as part of the test execution is the test itself (apart from our test setup, which is supposed to be side-effect free). Reviewed By: sammy-SC Differential Revision: D75681181 fbshipit-source-id: 91e4b903a49fcee59c5875e73db314cde0adea03
…1698) Summary: Pull Request resolved: facebook#51698 Changelog: [Internal] TSIA Reviewed By: hoxyq Differential Revision: D75682786 fbshipit-source-id: 3d70c36f26d87f0be003784aa76bec34640b38d2
Summary: Pull Request resolved: facebook#51699 Re-expose **all `"./src/*"` paths via `package.json#exports`. Follows D72228547 — as we'd attempted to be proactive with hiding `src/`, but are now reverting — essentially reducing this change as much as possible (read: most defensive/safer effect on OSS). **Motivation** Mitigates a warning emitted by Metro that may surface in new bare React Native template projects. {F1978757796} This is due to a 1P reference to `'react-native/src/private/featureflags/ReactNativeFeatureFlags` in `react-native/virtualized-lists`. This is enough motivation to undo part of our change with introducing `"exports"` on `react-native` in 0.80. **Especially**, to avoid confusion with other warnings we've intentionally introduced in 0.80 around subpath imports. - The key difference is the above screenshot is from Metro and covers **any** import from any `node_modules` file — as opposed to just the immediate project. Changelog: [General][Changed] - Re-expose `src/*` subpaths when not using the Strict TypeScript API The net breaking change for 0.80, once picked, is simply the presence of `"exports"` (only very edge case effects on expanding per-platform extensions, which we've mitigated). **All exported paths equivalent**. Reviewed By: robhogan Differential Revision: D75682566 fbshipit-source-id: f90c7298279c6be3a4eab70f2dfdc618ffcf1124
…ok#51702) Summary: Pull Request resolved: facebook#51702 We have no need for C++ bridging in this TurboModule, as the generated code matches the base codegen exactly. Changelog: [Internal] Reviewed By: rubennorte Differential Revision: D75681901 fbshipit-source-id: 4b6caafe0bcd08a06686e0a44112bdb99a64a1cf
Summary: Pull Request resolved: facebook#51703 Feedback on D75516619 Changelog: [Internal] Reviewed By: rubennorte Differential Revision: D75544422 fbshipit-source-id: f6c5e0090946b7861fd9cd9646e1b06b7c6a05d2
Summary: Pull Request resolved: facebook#51704 Changelog: [internal] If you create a Fantom test that throws an error during its setup (not during the execution of specific tests), Fantom fails with an error saying there are no tests defined, which isn't useful. This fixes the problem and shows where the error in the test setup happened exactly. Reviewed By: javache Differential Revision: D75689283 fbshipit-source-id: 54dd2382868dda0d284f46743ed94ba94aa3afdc
Summary: Pull Request resolved: facebook#51705 changelog: [internal] use if/else instead of unordered_map to map types of nodes/drivers from string to a class. This is faster and improves binary size. Reviewed By: javache Differential Revision: D75676701 fbshipit-source-id: be9b8b646ebd9472382e6f692768b8fe9703d88f
Summary: Pull Request resolved: facebook#51717 Changelog: [Internal] Reviewed By: gkz Differential Revision: D75715829 fbshipit-source-id: 8074481b11bfd28817b1caf315c8009cf6ae3807
Summary: The Image component in Android now supports the same cache control behaviour as in iOS, the examples needed to be separated in the past because the support was not the same for iOS and Android, but now that it is, we can unify the example. ## Changelog: [INTERNAL] - Unify RNTester Cache Policy Image example Pull Request resolved: facebook#51580 Test Plan: <details> <summary>RNTester Screenshots</summary> | iOS | Android | |--------|-------| |  |  | </details> Reviewed By: yungsters Differential Revision: D75538812 Pulled By: Abbondanzo fbshipit-source-id: 7705c8f824cb18cebc39e84b6f48979035dc104c
Summary: D75596329 disabled many, many rules that we previously explicitly enabled over the codebase, due to a specific rule causing crash in linter when specifically linting some files like RCTDeviceInfo.mm. Let's revert the change, and remove the specific problematic rule from our list of manually enabled rules instead. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D75734557 fbshipit-source-id: 9d6ee401bfa5d1efc8de993d366839c740aa0bdc
Summary: The retry mechanism introduced in [this commit]() works for iOS e2e failures but it is skipped if android e2e tests fails. This change should fix that ## Changelog: [Internal] - Fix retry for Android E2E tests Pull Request resolved: facebook#51684 Test Plan: GHA Reviewed By: rshest Differential Revision: D75719890 Pulled By: cipolleschi fbshipit-source-id: 0c2a22268bb655617eaf27f61227a46650878b4e
Summary: Fixes facebook#51548 ## 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 --> [General][ADDED] Added more Pending Decleration for ScrollView Pull Request resolved: facebook#51613 Test Plan: Can be tested on RNTester with ScrollView Reviewed By: cortinico Differential Revision: D75516801 Pulled By: rshest fbshipit-source-id: 87d6f68ab0a3ffd50af57b5eeaf313da4bf7ed98
…ok#51695) Summary: After testing the latest RC and nighly builds, crash appeared when emitting events from turbo modules on 32bit Android devices. The crash is always reproducible only on 32bit devices on signed production builds. Fore more details and the crash log, check the [related issue](facebook#51628 (comment)). From what I found, the variadic functions like CallVoidMethod are unsafe on 32bit due to not type checking the passed arguments at compile time. As far as I understand the 64bit cpus and ABIs are more forgiving with alignment and calling conventions. On 32bit the ABIs are strict as arguments are passed on the stack and if there is type/size/alignment issue it reads the wrong memory, which causes the SIGEGV crashes. ## Changelog: [ANDROID] [FIXED] - emitting event from turbo module crashes on 32bit android Pull Request resolved: facebook#51695 Test Plan: 1. Pull the [reproduction demo](https://github.com/vladimirivanoviliev/rn079eventcrash), install the dependencies (v `0.80` is on PR) 2. Run codegen on android 3. Build signed apk. To create it you will need to create new demo key-store. 4. To install the build apk in 32bit mode you can use `adb -s YOURDEVICE install --abi armeabi-v7a android/app/release/app-release.apk` 5. Run the app, create key, save it. Than update the key and save it again. The app crashes when try to emit event from the turbo module. 6. Patch the related `JavaTurboModule.cpp` file with the changes from this PR and enable build from source. 7. Rebuild and reinstall the apk and test again - the issue is now fixed ## Additional notes: I have tested the app on android using the `rn-tester` demo app, everything works as expected. I also patched our production app and tested more complex scenarios and they works as expected. I have run the tests and linter and they passed. One thing that I didn't able to setup and run is the iOS `rn-tester` app, due to Hermes engine error `Command PhaseScriptExecution failed with a nonzero exit code`. I haven't found any information how to fix it. I have followed [this guide](https://github.com/facebook/react-native/blob/main/packages/rn-tester/README.md) and installed node modules using yarn and started the `yarn prepare-ios`. I also haven't found any information with what node version and ruby version the react native package is build on CI so I use the same versions locally. If you provide me with updated instructions for those I can contribute by updating the related guides and including `.npmrc`, `.ruby-version` files. Reviewed By: cortinico Differential Revision: D75782377 Pulled By: javache fbshipit-source-id: b94998be6dd51e90ad4137b1d2e38a6850bc3cb2
Summary: Pull Request resolved: facebook#51739 Changelog: [Internal] - Update `react-native/debugger-frontend` from 343405b...41bf86b Resyncs `react-native/debugger-frontend` from GitHub - see `rn-chrome-devtools-frontend` [changelog](facebook/react-native-devtools-frontend@343405b...41bf86b). Reviewed By: huntie Differential Revision: D75785047 fbshipit-source-id: 5b2a611237e78811f140e8f3cdb8dec4a8b8a2be
Summary: Pull Request resolved: facebook#51707 Currently, the feature flag system does not properly support non-boolean feature flags. The type definitions and generated code make assumptions about the value type being `boolean`. This diff fixes these assumptions so that non-boolean feature flags are supported. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D75690997 fbshipit-source-id: 870063ff979d0650ce6d0a2f6b340d97c28d7d0b
Summary: Pull Request resolved: facebook#51708 Changes `yarn featureflags` to default to `--update` when no options are supplied. Provides a way to display the available options with a new `--help` option. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D75692119 fbshipit-source-id: 50d25c5f12c7646159872661c93ebe8c42f44788
Summary: Pull Request resolved: facebook#50317 `rncore`, `FBReactNativeSpec` and `FBReactNativeComponentSpec` contain the same symbols, which leads to conflicts when we try to merge them into a single shared library. Cleanup the duplication and standardize on `FBReactNativeSpec` everywhere. I've left the Android OSS targets names as is, to avoid breaking deps. This aligns react-native's package.json with the codegen tooling supported across iOS and Android, which is a single target for all all type-derived codegen. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D55037569 fbshipit-source-id: dbf3c0a427c9d0df96e439b04e5b123cd1069c51
Summary: Pull Request resolved: facebook#51743 Noticed some of these were missing, which may lose us useful test signal, or prevent tests from being ran at all. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D75789456 fbshipit-source-id: 75645866c672c77d3dac34383105955ef6d25e60
…d infinite animation loop workaround Summary: ## Changelog: [General] [Changed] - Remove native animation fabric sync in JS and infinite animation loop workaround, when cxxNativeAnimated is enabled when cxxNativeAnimated is enabled, we'll sync native animation props back to Fabric in native, when that happens we can remove the code in JS for same purpose Reviewed By: sammy-SC Differential Revision: D75789100 fbshipit-source-id: 966e2b187f43e8743ccbf7ba97b8e8a27273fe0c
Summary: - This is similar to facebook#43566, but include also updated `StackFrame` type. The current `Devtools.d.ts` doesn't match the `parseErrorStack.js` and `symbolicateStackTrace.js` implementations. I've tried to run `build-types`, but only `symbolicateStackTrace.d.ts` was generated. I've not checked why. ## 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 --> [GENERAL] [FIXED] - Devtools TS Types Pull Request resolved: facebook#51737 Test Plan: Call `parseErrorStack` and `symbolicateStackTrace` in TS. Reviewed By: rshest Differential Revision: D75782013 Pulled By: huntie fbshipit-source-id: 91fe560f079731af2a5834c8de8eafb723d00bf9
…ild flag for testing (facebook#51745) Summary: Pull Request resolved: facebook#51745 Changelog: [Internal] 1. Updates debugger-frontend's sync-and-build script to include a simple Markdown changelog inline and -mention authors who are Meta employees. 2. Also adds a `--no-build` flag that is mainly helpful for iterating on changelog generation logic. Reviewed By: hoxyq Differential Revision: D75789680 fbshipit-source-id: b30b49a9c50e93e7161a2dad012b92ef124a3a16
Summary: Pull Request resolved: facebook#51746 I observed that when continue-on-error is set to true, Github reports the outcome of a job as success even if it fails. With this change, we should ensure that when an E2E test fails, the pipeline fails, so that we can retry the jobs properly. ## Changelog: [Internal] - Remove continue-on-error from e2e tests in GHA Reviewed By: cortinico Differential Revision: D75796284 fbshipit-source-id: 0e769f53d7355ae6c985aace334b23205780673a
Summary: I'm writing some tests for some APIs/components, and I see some opportunity for improvement in the way how we can write e2e cases for the RNTester. This diff improves two things: 1. e2e execution times: right now as we are using the `scrollUntilVisible` functionality, it takes quite some time *for some cases*, as some of the items are not very up in the lists and getting to them it's not always very fast. 2. Flakiness: I ran the tests multiple times locally, and in multiple occasions, the `scrollUntilVisible ` did not find anything as the scroll was too fast so the test ended up failing. Instead of using `scrollUntilVisible` for all cases, we can simply use the search bar which we have in both Components and APIs tabs. This runs faster and we can also share the search flow across multiple test cases, so writing the tests becomes a bit simpler as well. Initially, I did this for all cases but not all cases benefit from this change as some of them are easier to find than others – the improvement was most notable in iOS (keyboard visibility seems to make a big difference), but I still think it is a good baseline to use the search as if more test cases are added, likely, many of them are not going to be so easy to find. ## Changelog: [INTERNAL] - Improve e2e times and flakiness by using the search Pull Request resolved: facebook#51590 Test Plan: These are the differences in the time it takes to pass the tests locally. iOS: | Before | After | |--------|-------| | <img width="336" alt="image" src="https://github.com/user-attachments/assets/6bc80de6-5a10-4c0c-b6b0-f7850d746781" /> | <img width="329" alt="image" src="https://github.com/user-attachments/assets/de47a7ff-84cf-4916-abce-9b3df96a03e1" /> | <details> <summary>Android (no significant difference):</summary> | Before | After | |--------|-------| | <img width="328" alt="image" src="https://github.com/user-attachments/assets/73023669-af8e-410c-aee2-529d1cce7acf" /> | <img width="336" alt="image" src="https://github.com/user-attachments/assets/3fb868a9-36f5-43c3-9261-6f2753e28e5f" /> | </details> Reviewed By: cortinico Differential Revision: D75389138 Pulled By: cipolleschi fbshipit-source-id: cf9d11ab0f84c91eaa20ee5c4441766729925571
Summary: Following up the announcement made at AppJS, this change stops testing the legacy architecture in our CI ## Changelog: [Internal] - Stop testing the legacy architecture in CI Pull Request resolved: facebook#51738 Test Plan: waiting for GHA Reviewed By: cortinico Differential Revision: D75791359 Pulled By: cipolleschi fbshipit-source-id: cb3159338835f49589fa6f495cfb9f47750825fe
Summary: Pull Request resolved: facebook#51755 this diff renames root -> view in ViewManager parameters changelog: [internal] internal Reviewed By: mlord93 Differential Revision: D75803186 fbshipit-source-id: e70708b5b504c13d617be3becac340d65471e67c
Summary: Pull Request resolved: facebook#51713 Changes the React Native Feature Flags system so that numeric feature flags are represented in native languages as double instead of int, in order to avoid loss of precision for non-integral JavaScript numbers. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D75709111 fbshipit-source-id: d4b2d553ce1e719a5f4b136a4f8d2e46446ef8d8
Summary: Pull Request resolved: facebook#51716 Adds a feature flag to experiment with different `prerenderRatio` values from the `VirtualView` native component. Notably, this is the first time that the a feature flag has a non-boolean type. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D75713594 fbshipit-source-id: ff3feb8d8bb3292fea6e04a4cfccd87e8249144e
Summary: This PR adds ImageSource type to ImageSource.d.ts which is defined in Flow: https://github.com/facebook/react-native/blob/d6f29c8afd14b2cc835649db3c59ed2f0e685331/packages/react-native/Libraries/Image/ImageSource.js#L87-L90 But not in the TypeScript file. ## Changelog: [GENERAL] [FIXED] - add ImageSource type to TypeScript Pull Request resolved: facebook#51969 Test Plan: CI Green Reviewed By: fabriziocucci Differential Revision: D76532377 Pulled By: Abbondanzo fbshipit-source-id: f1bbcd3b3fc07bb0f7e82f81ebaffedf9bc06148
Summary: Pull Request resolved: facebook#51966 This starts off mechanically, but needed a couple changes: 1. Some null handling changes to `TextTransform` internals 2. We type MapBuffer keys as `Int` instead of `Short`, because Kotlin does not allow the implicit widening cast that Java does. I also made these internal 3. Some shifts around casting 4. Mark TextLayoutManager internal, and remove usages of `UnstableReactNativeAPI` I verified that there were no usages of the Java side of TextLayoutManager throughout `react-native-libraries`, so marking TextLayoutManager internal is unlikely to break 3p libraries. Changelog: [Android][Breaking] - Make Java Side TextLayoutManager Internal Reviewed By: javache Differential Revision: D76444163 fbshipit-source-id: aabb1c498c731598559f0df5c12e0ecdc266339f
Summary: FIXED Add index.js.flow to npm package files for Flow support Currently, the distributed npm package for react-native does not include the index.js.flow file, which causes all exports to be typed as any when using Flow. This commit adds index.js.flow to the "files" array in package.json, ensuring Flow users receive proper type definitions out of the box. This addresses issues where type checking with Flow fails in React Native projects. ## Changelog: [General][Added] Publish top-level Flow types for `react-native` Pull Request resolved: facebook#51908 Reviewed By: huntie, necolas Differential Revision: D76292301 Pulled By: robhogan fbshipit-source-id: e56360d3f35af30ef160470181349aac1812e7c1
…ative-github/packages/react-native/ReactCommon/react/renderer/graphics/platform/ios/react/renderer/graphics (facebook#51983) Summary: Pull Request resolved: facebook#51983 Reviewed By: dtolnay Differential Revision: D76494516 fbshipit-source-id: 399311ad4e1eadf6741926a19ce1919e73a1bdaa
…OS (facebook#51770) Summary: This change allows for dashed and dotted borders to have different widths for each of the sides on iOS. This issue was described in facebook#51658. This allows for better dashed lines and moves the implementation of borders closer to how it is handled on web/android. Resolves facebook#51658 (related facebook#39088) ## Changelog: [IOS] [ADDED] - Add support for different borderWidths Pull Request resolved: facebook#51770 Test Plan: - yarn test - yarn lint Reviewed By: NickGerleman Differential Revision: D76145887 Pulled By: jorge-cab fbshipit-source-id: 3716e84799b44d2ff0994cc673a2172ee85bd9e6
Summary: Pull Request resolved: facebook#51987 In this diff I'm adding flowTypes for codegen LIBRARY_GENERATORS changelog: [internal] internal Reviewed By: huntie Differential Revision: D76470808 fbshipit-source-id: 8e2bddeda1f9175fd25fee04f8fdd3cb7c7faa49
Summary: Pull Request resolved: facebook#51990 In this diff I'm limiting visibility of internal objects of codegen, these objects are being exported but they are unused, let's avoid exporting them changelog: [internal] internal Reviewed By: christophpurrer Differential Revision: D76470809 fbshipit-source-id: 0e168558d2d3211ab5a3a3de05e2495d7c1ae4f5
facebook#51991) Summary: Pull Request resolved: facebook#51991 This diff introduces a new parameter to customize libraryGenerators used in the codegen, since I'm adding a default object, this diff shoulnd't change any behavior changelog: [internal] internal Reviewed By: christophpurrer Differential Revision: D76472495 fbshipit-source-id: 50b9095c7c554e368f65e4c0b5539be0cca51a51
…ook#51972) Summary: This diff adds snapshot `diff-api-snapshot` script for public JS API breaking change detection. ### Motivation Detecting if there are any breaking changes introduced in the commit. It is achieved by comparing `ReactNativeApi.d.ts` rollup from the current and previous revision. This is a naive implementation with a three possible outcomes: - BREAKING - POTENTIALLY_NOT_BREAKING, - NOT_BREAKING The algorithm analyses exported top-level statements (after inlining) in both rollups and tries to create a mapping between them by name. The **BREAKING** outcome happens whenever the statement is: - removed - renamed - changed - not exported anymore (private) The **POTENTIALLY_NOT_BREAKING** outcome happens if it's not BREAKING and the new statement is added. The **NOT_BREAKING** outcome happens if public API snapshot doesn't change. Changelog: [General][Added] - Add public JS API breaking change detection under `yarn diff-api-snapshot` script. Pull Request resolved: facebook#51972 Test Plan: Imported from GitHub, without a `Test Plan:` line. Rollback Plan: Differential Revision: D76430965 Pulled By: coado
coado
pushed a commit
that referenced
this pull request
Jun 16, 2025
Summary: Pull Request resolved: facebook#51425 # Problem React native's new architecture will allow components to do sync render/events. That means they'll makes synchronous dispatches from main thread to the js thread, to capture the runtime so that they can execute js on the main thread. But, the js thread already as a bunch of synchronous calls to the main thread. So, if any of those js -> ui sync calls happen concurrently with a synchronous render, the application will deadlock. This diff is an attempt to mitigate all those deadlocks. ## Context How js execution from the main thread works: * Main thread puts a block on the js thread, to capture the js runtime. Main thread is put to sleep. * Js thread executes "runtime capture block". The runtime is captured for the main thread. The js thread is put to sleep. * Main thread wakes up, noticing that the runtime is captured. It executes its js code with the captured runtime. Then, it releases the runtime, which wakes up the js thread. Both the main and js thread move on to other tasks. How synchronous js -> main thread calls work: * Js thread puts a ui block on the main queue. * Js thread goes to sleep until that ui block executes on the main thread. ## Deadlock #1 **Main thread**: execute js now: * Main thread puts a block on the js queue, to capture the runtime. * Main thread then then goes to sleep, waiting for runtime to be captured **JS thread**: execute ui code synchronously: * Js thread schedules a block on the ui thread * Js thread then goes to sleep, waiting for that block to execute. **Result:** The application deadlocks | {F1978009555} | {F1978009612} |  ## Deadlock #2 **JS thread**: execute ui code synchronously: * Js thread schedules a block on the ui thread * Js thread then goes to sleep waiting for that block to execute. **Main thread**: execute js now: * Main thread puts a block on the js queue, to capture the runtime. * Main thread then then goes to sleep, waiting for runtime to be captured **Result:** The application deadlocks | {F1978009690} | {F1978009701} |  # Changes This diff attempts to fix those deadlocks. How: * In "execute ui code synchronously" (js thread): * Before going to sleep, the js thread schedules the ui work on the main queue, **and** it posts the ui work to "execute js now". * In "execute js now" (main thread): * This diff makes "execute js now" stateful: it keeps a "pending ui block." * Before capturing the runtime, the "execute js now" executes "pending ui work", if it exists. * While sleeping waiting for runtime capture, "execute js now" can wake up, and execute "pending ui work." It goes back to sleep afterwards, waiting for runtime capture. ## Mitigation: Deadlock #1 **Main thread**: execute js now: * Main thread puts a block on the js queue, to capture the runtime. * Main thread then then goes to sleep, waiting for runtime capture **JS Thread**: execute ui code synchronously: * Js thread puts its ui block on the ui queue. * ***New***: Js thread also posts that ui block to "execute js now". Main thread was sleeping waiting for runtime to be captured. It now wakes up. * Js thread goes to sleep. The main thread wakes up in "execute js now": * Main thread sees that a "pending ui block" is posted. It executes the "pending ui block." The block, also scheduled on the main thread, noops henceforth. * Main thread goes back to sleep, waiting for runtime capture. * The js thread wakes up, moves on to the next task. **Result:** The runtime is captured by the main thread. | {F1978010383} | {F1978010363} | {F1978010371} | {F1978010379} |  ## Mitigation: Deadlock #2 **JS Thread**: execute ui code synchronously: * Js thread puts its ui block on the ui queue. * ***New***: Js thread also posts that ui block to "execute js now". Main thread was sleeping waiting for runtime to be captured. It now wakes up. * Js thread goes to sleep. **Main thread**: execute js now * Main thread sees that a "pending ui block" is posted. It executes the "pending ui block" immediately. The block, also scheduled on the main thread, noops henceforth. * Js thread wakes up and moves onto the next task. **Result:** Main thread captures the runtime. | {F1978010525} | {F1978010533} | {F1978010542} |  Changelog: [Internal] Reviewed By: javache Differential Revision: D74769326 fbshipit-source-id: 854b83ce4e482a4030dc711834ea6c5613091537
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Changelog:
Test Plan: