Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement re-entrance and unlocked scenarios for LazyShadowTreeRevisionConsistencyManager #44620

Closed
wants to merge 2 commits into from

Conversation

rubennorte
Copy link
Contributor

Summary:
We added a log message when trying to lock revisions in LazyShadowTreeRevisionConsistencyManager when they were already locked, and we've seen that being logged in existing experiments, which could indicate we're doing re-entrance from the JS runtime.

This protects against that case migrating the boolean flag to an integer.

Differential Revision: D57509193

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

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

…cyManager (facebook#44619)

Summary:

Some methods in `LazyShadowTreeRevisionConsistencyManager` can be called in parallel when using synchronous state updates (which is also behind a flag). This implements thread-safety to cover that case so we don't have issues when testing that variant in production.

Changelog: [internal]

Reviewed By: NickGerleman

Differential Revision: D57506540
…onConsistencyManager (facebook#44620)

Summary:

We added a log message when trying to lock revisions in `LazyShadowTreeRevisionConsistencyManager` when they were already locked, and we've seen that being logged in existing experiments, which could indicate we're doing re-entrance from the JS runtime. 

This protects against that case migrating the boolean flag to an integer.

Differential Revision: D57509193
@facebook-github-bot
Copy link
Contributor

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

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 19,542,752 +8
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 22,912,804 -6
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 8d5bbca
Branch: main

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label May 22, 2024
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in b8f3478.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants