Skip to content

Commit

Permalink
Use a mutex to guard access to lastLayoutMetrics_ in ViewEventEmitter
Browse files Browse the repository at this point in the history
Summary:
Of course, compare_exchange_strong didn't actually do what I wanted.

Using a mutex is simpler and actually has the semantics we want: atomically get the current value, compare, and bail if the value is the same, or swap and continue.

Changelog: [Internal]

Reviewed By: shergin

Differential Revision: D19754451

fbshipit-source-id: 6b0aef217b235959af683ec5e31b07a0dd7bb040
  • Loading branch information
JoshuaGross authored and facebook-github-bot committed Feb 6, 2020
1 parent f555202 commit 6be37d8
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 8 deletions.
12 changes: 6 additions & 6 deletions ReactCommon/fabric/components/view/ViewEventEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@ void ViewEventEmitter::onLayout(const LayoutMetrics &layoutMetrics) const {
// Due to State Reconciliation, `onLayout` can be called potentially many
// times with identical layoutMetrics. Ensure that the JS event is only
// dispatched when the value changes.
auto lastLayoutMetricsValue = lastLayoutMetrics_.load();
// compare_exchange_strong atomically swap the values if they're different, or
// return false if the values are the same.
if (!lastLayoutMetrics_.compare_exchange_strong(
lastLayoutMetricsValue, layoutMetrics)) {
return;
{
std::lock_guard<std::mutex> guard(layoutMetricsMutex_);
if (lastLayoutMetrics_ == layoutMetrics) {
return;
}
lastLayoutMetrics_ = layoutMetrics;
}

dispatchEvent("layout", [frame = layoutMetrics.frame](jsi::Runtime &runtime) {
Expand Down
5 changes: 3 additions & 2 deletions ReactCommon/fabric/components/view/ViewEventEmitter.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@

#pragma once

#include <atomic>
#include <memory>
#include <mutex>

#include <react/core/LayoutMetrics.h>
#include <react/core/ReactPrimitives.h>
Expand Down Expand Up @@ -38,7 +38,8 @@ class ViewEventEmitter : public TouchEventEmitter {
void onLayout(const LayoutMetrics &layoutMetrics) const;

private:
mutable std::atomic<LayoutMetrics> lastLayoutMetrics_;
mutable std::mutex layoutMetricsMutex_;
mutable LayoutMetrics lastLayoutMetrics_;
};

} // namespace react
Expand Down

0 comments on commit 6be37d8

Please sign in to comment.