Skip to content

Commit

Permalink
Improve thread safety of ReactNativeFeatureFlags (#42870)
Browse files Browse the repository at this point in the history 
Summary:
Pull Request resolved: #42870

This refactors `ReactNativeFeatureFlagsAccessor` in C++ to improve its thread-safety:
* It makes all cached feature flags atomic to prevent data corruption when writing them concurrently.
* It refactors the list of accessed feature flags to be an array of atomic character pointers instead of a vector.

Performance-wise, this is lock-free so it would still be fast enough for our use cases.

Semantic-wise, this implementation could lead to feature flags being initialized more than once (if 2 threads happen to access the same feature flag before it has been initialized), but that's ok. The only consequence of this would be accessing the provider twice, but the end state of the accessor is the same (the same value would be cached and the flag would still be marked as accessed).

Changelog: [internal]

Reviewed By: javache

Differential Revision: D53406924

fbshipit-source-id: 1023673c40f9da43a51c5f96354d4c458c9d14d4
  • Loading branch information
@rubennorte @facebook-github-bot
rubennorte authored and facebook-github-bot committed Feb 5, 2024
1 parent f3977af commit fe69f71
Show file tree
Hide file tree
Showing 6 changed files with 176 additions and 110 deletions.
5 changes: 3 additions & 2 deletions packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<d3e8b0cc31fd8e44c3f83d836485e6f6>>
* @generated SignedSource<<252f46e2c95e08d1180f656c870087b7>>
*/

/**
Expand All @@ -28,7 +28,8 @@ namespace facebook::react {
/**
* This class provides access to internal React Native feature flags.
*
* All the methods are thread-safe if you handle `override` correctly.
* All the methods are thread-safe (as long as the methods in the overridden
* provider are).
*/
class ReactNativeFeatureFlags {
public:
Expand Down
183 changes: 109 additions & 74 deletions packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<7d50752c48503e31eccfe64d66ee2e99>>
* @generated SignedSource<<ad8bfca49acda85e56fea1c124b5bc8a>>
*/

/**
Expand All @@ -29,143 +29,178 @@ ReactNativeFeatureFlagsAccessor::ReactNativeFeatureFlagsAccessor()
: currentProvider_(std::make_unique<ReactNativeFeatureFlagsDefaults>()) {}

bool ReactNativeFeatureFlagsAccessor::commonTestFlag() {
if (!commonTestFlag_.has_value()) {
auto flagValue = commonTestFlag_.load();

if (!flagValue.has_value()) {
// This block is not exclusive but it is not necessary.
// If multiple threads try to initialize the feature flag, we would only
// be accessing the provider multiple times but the end state of this
// instance and the returned flag value would be the same.

// Mark the flag as accessed.
static const char* flagName = "commonTestFlag";
if (std::find(
accessedFeatureFlags_.begin(),
accessedFeatureFlags_.end(),
flagName) == accessedFeatureFlags_.end()) {
accessedFeatureFlags_.push_back(flagName);
}
markFlagAsAccessed(0, flagName);

commonTestFlag_.emplace(currentProvider_->commonTestFlag());
flagValue = currentProvider_->commonTestFlag();
commonTestFlag_ = flagValue;
}

return commonTestFlag_.value();
return flagValue.value();
}

bool ReactNativeFeatureFlagsAccessor::useModernRuntimeScheduler() {
if (!useModernRuntimeScheduler_.has_value()) {
auto flagValue = useModernRuntimeScheduler_.load();

if (!flagValue.has_value()) {
// This block is not exclusive but it is not necessary.
// If multiple threads try to initialize the feature flag, we would only
// be accessing the provider multiple times but the end state of this
// instance and the returned flag value would be the same.

// Mark the flag as accessed.
static const char* flagName = "useModernRuntimeScheduler";
if (std::find(
accessedFeatureFlags_.begin(),
accessedFeatureFlags_.end(),
flagName) == accessedFeatureFlags_.end()) {
accessedFeatureFlags_.push_back(flagName);
}
markFlagAsAccessed(1, flagName);

useModernRuntimeScheduler_.emplace(currentProvider_->useModernRuntimeScheduler());
flagValue = currentProvider_->useModernRuntimeScheduler();
useModernRuntimeScheduler_ = flagValue;
}

return useModernRuntimeScheduler_.value();
return flagValue.value();
}

bool ReactNativeFeatureFlagsAccessor::enableMicrotasks() {
if (!enableMicrotasks_.has_value()) {
auto flagValue = enableMicrotasks_.load();

if (!flagValue.has_value()) {
// This block is not exclusive but it is not necessary.
// If multiple threads try to initialize the feature flag, we would only
// be accessing the provider multiple times but the end state of this
// instance and the returned flag value would be the same.

// Mark the flag as accessed.
static const char* flagName = "enableMicrotasks";
if (std::find(
accessedFeatureFlags_.begin(),
accessedFeatureFlags_.end(),
flagName) == accessedFeatureFlags_.end()) {
accessedFeatureFlags_.push_back(flagName);
}
markFlagAsAccessed(2, flagName);

enableMicrotasks_.emplace(currentProvider_->enableMicrotasks());
flagValue = currentProvider_->enableMicrotasks();
enableMicrotasks_ = flagValue;
}

return enableMicrotasks_.value();
return flagValue.value();
}

bool ReactNativeFeatureFlagsAccessor::batchRenderingUpdatesInEventLoop() {
if (!batchRenderingUpdatesInEventLoop_.has_value()) {
auto flagValue = batchRenderingUpdatesInEventLoop_.load();

if (!flagValue.has_value()) {
// This block is not exclusive but it is not necessary.
// If multiple threads try to initialize the feature flag, we would only
// be accessing the provider multiple times but the end state of this
// instance and the returned flag value would be the same.

// Mark the flag as accessed.
static const char* flagName = "batchRenderingUpdatesInEventLoop";
if (std::find(
accessedFeatureFlags_.begin(),
accessedFeatureFlags_.end(),
flagName) == accessedFeatureFlags_.end()) {
accessedFeatureFlags_.push_back(flagName);
}
markFlagAsAccessed(3, flagName);

batchRenderingUpdatesInEventLoop_.emplace(currentProvider_->batchRenderingUpdatesInEventLoop());
flagValue = currentProvider_->batchRenderingUpdatesInEventLoop();
batchRenderingUpdatesInEventLoop_ = flagValue;
}

return batchRenderingUpdatesInEventLoop_.value();
return flagValue.value();
}

bool ReactNativeFeatureFlagsAccessor::enableSpannableBuildingUnification() {
if (!enableSpannableBuildingUnification_.has_value()) {
auto flagValue = enableSpannableBuildingUnification_.load();

if (!flagValue.has_value()) {
// This block is not exclusive but it is not necessary.
// If multiple threads try to initialize the feature flag, we would only
// be accessing the provider multiple times but the end state of this
// instance and the returned flag value would be the same.

// Mark the flag as accessed.
static const char* flagName = "enableSpannableBuildingUnification";
if (std::find(
accessedFeatureFlags_.begin(),
accessedFeatureFlags_.end(),
flagName) == accessedFeatureFlags_.end()) {
accessedFeatureFlags_.push_back(flagName);
}
markFlagAsAccessed(4, flagName);

enableSpannableBuildingUnification_.emplace(currentProvider_->enableSpannableBuildingUnification());
flagValue = currentProvider_->enableSpannableBuildingUnification();
enableSpannableBuildingUnification_ = flagValue;
}

return enableSpannableBuildingUnification_.value();
return flagValue.value();
}

bool ReactNativeFeatureFlagsAccessor::enableCustomDrawOrderFabric() {
if (!enableCustomDrawOrderFabric_.has_value()) {
auto flagValue = enableCustomDrawOrderFabric_.load();

if (!flagValue.has_value()) {
// This block is not exclusive but it is not necessary.
// If multiple threads try to initialize the feature flag, we would only
// be accessing the provider multiple times but the end state of this
// instance and the returned flag value would be the same.

// Mark the flag as accessed.
static const char* flagName = "enableCustomDrawOrderFabric";
if (std::find(
accessedFeatureFlags_.begin(),
accessedFeatureFlags_.end(),
flagName) == accessedFeatureFlags_.end()) {
accessedFeatureFlags_.push_back(flagName);
}
markFlagAsAccessed(5, flagName);

enableCustomDrawOrderFabric_.emplace(currentProvider_->enableCustomDrawOrderFabric());
flagValue = currentProvider_->enableCustomDrawOrderFabric();
enableCustomDrawOrderFabric_ = flagValue;
}

return enableCustomDrawOrderFabric_.value();
return flagValue.value();
}

bool ReactNativeFeatureFlagsAccessor::enableFixForClippedSubviewsCrash() {
if (!enableFixForClippedSubviewsCrash_.has_value()) {
auto flagValue = enableFixForClippedSubviewsCrash_.load();

if (!flagValue.has_value()) {
// This block is not exclusive but it is not necessary.
// If multiple threads try to initialize the feature flag, we would only
// be accessing the provider multiple times but the end state of this
// instance and the returned flag value would be the same.

// Mark the flag as accessed.
static const char* flagName = "enableFixForClippedSubviewsCrash";
if (std::find(
accessedFeatureFlags_.begin(),
accessedFeatureFlags_.end(),
flagName) == accessedFeatureFlags_.end()) {
accessedFeatureFlags_.push_back(flagName);
}
markFlagAsAccessed(6, flagName);

enableFixForClippedSubviewsCrash_.emplace(currentProvider_->enableFixForClippedSubviewsCrash());
flagValue = currentProvider_->enableFixForClippedSubviewsCrash();
enableFixForClippedSubviewsCrash_ = flagValue;
}

return enableFixForClippedSubviewsCrash_.value();
return flagValue.value();
}

void ReactNativeFeatureFlagsAccessor::override(
std::unique_ptr<ReactNativeFeatureFlagsProvider> provider) {
if (!accessedFeatureFlags_.empty()) {
std::ostringstream featureFlagListBuilder;
for (const auto& featureFlagName : accessedFeatureFlags_) {
ensureFlagsNotAccessed();
currentProvider_ = std::move(provider);
}

void ReactNativeFeatureFlagsAccessor::markFlagAsAccessed(
int position,
const char* flagName) {
accessedFeatureFlags_[position] = flagName;
}

void ReactNativeFeatureFlagsAccessor::ensureFlagsNotAccessed() {
std::string accessedFeatureFlagNames;

std::ostringstream featureFlagListBuilder;
for (const auto& featureFlagName : accessedFeatureFlags_) {
if (featureFlagName != nullptr) {
featureFlagListBuilder << featureFlagName << ", ";
}
std::string accessedFeatureFlagNames = featureFlagListBuilder.str();
if (!accessedFeatureFlagNames.empty()) {
accessedFeatureFlagNames = accessedFeatureFlagNames.substr(
0, accessedFeatureFlagNames.size() - 2);
}
}

accessedFeatureFlagNames = featureFlagListBuilder.str();
if (!accessedFeatureFlagNames.empty()) {
accessedFeatureFlagNames =
accessedFeatureFlagNames.substr(0, accessedFeatureFlagNames.size() - 2);
}

if (!accessedFeatureFlagNames.empty()) {
throw std::runtime_error(
"Feature flags were accessed before being overridden: " +
accessedFeatureFlagNames);
}

currentProvider_ = std::move(provider);
}

} // namespace facebook::react
25 changes: 15 additions & 10 deletions packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<1514c04fb9d175308e106b84a14bc89d>>
* @generated SignedSource<<40315ae6cc418effd421dff2be6831c7>>
*/

/**
Expand All @@ -20,6 +20,8 @@
#pragma once

#include <react/featureflags/ReactNativeFeatureFlagsProvider.h>
#include <array>
#include <atomic>
#include <memory>
#include <optional>
#include <vector>
Expand All @@ -41,16 +43,19 @@ class ReactNativeFeatureFlagsAccessor {
void override(std::unique_ptr<ReactNativeFeatureFlagsProvider> provider);

private:
void markFlagAsAccessed(int position, const char* flagName);
void ensureFlagsNotAccessed();

std::unique_ptr<ReactNativeFeatureFlagsProvider> currentProvider_;
std::vector<const char*> accessedFeatureFlags_;

std::optional<bool> commonTestFlag_;
std::optional<bool> useModernRuntimeScheduler_;
std::optional<bool> enableMicrotasks_;
std::optional<bool> batchRenderingUpdatesInEventLoop_;
std::optional<bool> enableSpannableBuildingUnification_;
std::optional<bool> enableCustomDrawOrderFabric_;
std::optional<bool> enableFixForClippedSubviewsCrash_;
std::array<std::atomic<const char*>, 7> accessedFeatureFlags_;

std::atomic<std::optional<bool>> commonTestFlag_;
std::atomic<std::optional<bool>> useModernRuntimeScheduler_;
std::atomic<std::optional<bool>> enableMicrotasks_;
std::atomic<std::optional<bool>> batchRenderingUpdatesInEventLoop_;
std::atomic<std::optional<bool>> enableSpannableBuildingUnification_;
std::atomic<std::optional<bool>> enableCustomDrawOrderFabric_;
std::atomic<std::optional<bool>> enableFixForClippedSubviewsCrash_;
};

} // namespace facebook::react
3 changes: 2 additions & 1 deletion ...ct-native/scripts/featureflags/templates/common-cxx/ReactNativeFeatureFlags.h-template.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ namespace facebook::react {
/**
* This class provides access to internal React Native feature flags.
*
* All the methods are thread-safe if you handle \`override\` correctly.
* All the methods are thread-safe (as long as the methods in the overridden
* provider are).
*/
class ReactNativeFeatureFlags {
public:
Expand Down
Loading

0 comments on commit fe69f71

Please sign in to comment.