From 6be37d8c0a2057021fdeb761eee7ac1567dbde54 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Wed, 5 Feb 2020 18:43:51 -0800 Subject: [PATCH] Use a mutex to guard access to lastLayoutMetrics_ in ViewEventEmitter 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 --- .../fabric/components/view/ViewEventEmitter.cpp | 12 ++++++------ .../fabric/components/view/ViewEventEmitter.h | 5 +++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/ReactCommon/fabric/components/view/ViewEventEmitter.cpp b/ReactCommon/fabric/components/view/ViewEventEmitter.cpp index 20693ec0faad59..fe2a83f5f43d8d 100644 --- a/ReactCommon/fabric/components/view/ViewEventEmitter.cpp +++ b/ReactCommon/fabric/components/view/ViewEventEmitter.cpp @@ -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 guard(layoutMetricsMutex_); + if (lastLayoutMetrics_ == layoutMetrics) { + return; + } + lastLayoutMetrics_ = layoutMetrics; } dispatchEvent("layout", [frame = layoutMetrics.frame](jsi::Runtime &runtime) { diff --git a/ReactCommon/fabric/components/view/ViewEventEmitter.h b/ReactCommon/fabric/components/view/ViewEventEmitter.h index 0920931a12f8a0..608755c5053e61 100644 --- a/ReactCommon/fabric/components/view/ViewEventEmitter.h +++ b/ReactCommon/fabric/components/view/ViewEventEmitter.h @@ -7,8 +7,8 @@ #pragma once -#include #include +#include #include #include @@ -38,7 +38,8 @@ class ViewEventEmitter : public TouchEventEmitter { void onLayout(const LayoutMetrics &layoutMetrics) const; private: - mutable std::atomic lastLayoutMetrics_; + mutable std::mutex layoutMetricsMutex_; + mutable LayoutMetrics lastLayoutMetrics_; }; } // namespace react