mirror of
https://github.com/facebook/react-native.git
synced 2025-11-01 09:14:26 +00:00
Fix incorrect locking and attempts check in ShadowTree experiment (#52681)
Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/52681 Changelog: [internal] In the original change I made in D78418504 / https://github.com/facebook/react-native/pull/52645 I made 2 mistakes: 1. Used a lock that would try to re-lock on itself without it being recursive (which would cause a deadlock). I didn't see that because when testing I didn't hit the case where we'd exhaust the options. 2. The `attemps` variable wasn't incremented, so we never left the loop in case of exhaustion. This propagates a flag to `tryCommit` to indicate we've already locked on the commitMutex_ so we don't need to lock again in that case and increases the counter, fixing the issue. Reviewed By: cortinico Differential Revision: D78497509 fbshipit-source-id: 546ccd0c84aed5416ce1aef47d79419b4fe06f66
This commit is contained in:
committed by
Nicola Corti
parent
c82821fe61
commit
8533d28f45
@@ -251,11 +251,12 @@ CommitStatus ShadowTree::commit(
|
||||
if (status != CommitStatus::Failed) {
|
||||
return status;
|
||||
}
|
||||
attempts++;
|
||||
}
|
||||
|
||||
{
|
||||
std::unique_lock lock(commitMutex_);
|
||||
return tryCommit(transaction, commitOptions);
|
||||
return tryCommit(transaction, commitOptions, true);
|
||||
}
|
||||
} else {
|
||||
while (true) {
|
||||
@@ -275,7 +276,8 @@ CommitStatus ShadowTree::commit(
|
||||
|
||||
CommitStatus ShadowTree::tryCommit(
|
||||
const ShadowTreeCommitTransaction& transaction,
|
||||
const CommitOptions& commitOptions) const {
|
||||
const CommitOptions& commitOptions,
|
||||
bool hasLocked) const {
|
||||
TraceSection s("ShadowTree::commit");
|
||||
|
||||
auto telemetry = TransactionTelemetry{};
|
||||
@@ -287,7 +289,10 @@ CommitStatus ShadowTree::tryCommit(
|
||||
|
||||
{
|
||||
// Reading `currentRevision_` in shared manner.
|
||||
std::shared_lock lock(commitMutex_);
|
||||
std::shared_lock lock(commitMutex_, std::defer_lock);
|
||||
if (!hasLocked) {
|
||||
lock.lock();
|
||||
}
|
||||
commitMode = commitMode_;
|
||||
oldRevision = currentRevision_;
|
||||
}
|
||||
@@ -328,7 +333,10 @@ CommitStatus ShadowTree::tryCommit(
|
||||
|
||||
{
|
||||
// Updating `currentRevision_` in unique manner if it hasn't changed.
|
||||
std::unique_lock lock(commitMutex_);
|
||||
std::unique_lock lock(commitMutex_, std::defer_lock);
|
||||
if (!hasLocked) {
|
||||
lock.lock();
|
||||
}
|
||||
|
||||
if (currentRevision_.number != oldRevision.number) {
|
||||
return CommitStatus::Failed;
|
||||
|
||||
@@ -111,7 +111,8 @@ class ShadowTree final {
|
||||
*/
|
||||
CommitStatus tryCommit(
|
||||
const ShadowTreeCommitTransaction& transaction,
|
||||
const CommitOptions& commitOptions) const;
|
||||
const CommitOptions& commitOptions,
|
||||
bool hasLocked = false) const;
|
||||
|
||||
/*
|
||||
* Calls `tryCommit` in a loop until it finishes successfully.
|
||||
|
||||
Reference in New Issue
Block a user