Add new dangerouslyForceOverride method to feature flags (facebook#46963

)

Summary:

Changelog: [internal]

This introduces a new method in `ReactNativeFeatureFlags` to force setting overrides without triggering any errors (they're returned a string instead to be handled in userland). This is partially equivalent to calling `dangerouslyReset` and `override` but completely avoids the hard crashes that could be caused by race conditions.

Reviewed By: javache, rshest

Differential Revision: D64186262

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt

@@ -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<<405d53cd6aba78616b8690c26a0accad>>
+ * @generated SignedSource<<e568acc70be10bdd8f86caddd03cde05>>
  */
 
 /**
@@ -358,6 +358,32 @@ public object ReactNativeFeatureFlags {
     accessor = accessorProvider()
   }
 
+  /**
+   * This is a combination of `dangerouslyReset` and `override` that reduces
+   * the likeliness of a race condition between the two calls.
+   *
+   * This is **dangerous** because it can introduce consistency issues that will
+   * be much harder to debug. For example, it could hide the fact that feature
+   * flags are read before you set the values you want to use everywhere. It
+   * could also cause a workflow to suddently have different feature flags for
+   * behaviors that were configured with different values before.
+   *
+   * It returns a string that contains the feature flags that were accessed
+   * before this call (or between the last call to `dangerouslyReset` and this
+   * call). If you are using this method, you do not want the hard crash that
+   * you would get from using `dangerouslyReset` and `override` separately,
+   * but you should still log this somehow.
+   *
+   * Please see the documentation of `dangerouslyReset` for additional details.
+   */
+  @JvmStatic
+  public fun dangerouslyForceOverride(provider: ReactNativeFeatureFlagsProvider): String? {
+    val newAccessor = accessorProvider()
+    val previouslyAccessedFlags = newAccessor.dangerouslyForceOverride(provider)
+    accessor = newAccessor
+    return previouslyAccessedFlags
+  }
+
   /**
    * This is just used to replace the default ReactNativeFeatureFlagsCxxAccessor
    * that uses JNI with a version that doesn't, to simplify testing.

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsAccessor.kt

@@ -11,4 +11,6 @@ public interface ReactNativeFeatureFlagsAccessor : ReactNativeFeatureFlagsProvid
   public fun override(provider: ReactNativeFeatureFlagsProvider)
 
   public fun dangerouslyReset()
+
+  public fun dangerouslyForceOverride(provider: ReactNativeFeatureFlagsProvider): String?
 }

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.kt

@@ -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<<84ee754f916b48a7c55ea94e166510c7>>
+ * @generated SignedSource<<a2a7b39f3f71be2a278faf2c21ede6a3>>
  */
 
 /**
@@ -515,4 +515,7 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
       ReactNativeFeatureFlagsCxxInterop.override(provider as Any)
 
   override fun dangerouslyReset(): Unit = ReactNativeFeatureFlagsCxxInterop.dangerouslyReset()
+
+  override fun dangerouslyForceOverride(provider: ReactNativeFeatureFlagsProvider): String? =
+      ReactNativeFeatureFlagsCxxInterop.dangerouslyForceOverride(provider as Any)
 }

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.kt

@@ -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<<eebeaa749dafac5d49d9d6b356f88817>>
+ * @generated SignedSource<<3cf8639dfd8a92a954a055c3ebdc749c>>
  */
 
 /**
@@ -129,4 +129,6 @@ public object ReactNativeFeatureFlagsCxxInterop {
   @DoNotStrip @JvmStatic public external fun override(provider: Any)
 
   @DoNotStrip @JvmStatic public external fun dangerouslyReset()
+
+  @DoNotStrip @JvmStatic public external fun dangerouslyForceOverride(provider: Any): String?
 }

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.kt

@@ -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<<4dc2364f5bcd765d7b61dbcab0d9533d>>
+ * @generated SignedSource<<91c4268e7e88e2e3c1f3d50d149c0d2b>>
  */
 
 /**
@@ -574,7 +574,21 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
   }
 
   override fun dangerouslyReset() {
-    // We don't need to do anything here because `ReactNativeFeatureFlags` will
-    // just create a new instance of this class.
+    // We don't need to do anything else here because `ReactNativeFeatureFlags` will just create a
+    // new instance of this class.
+  }
+
+  override fun dangerouslyForceOverride(provider: ReactNativeFeatureFlagsProvider): String? {
+    val accessedFeatureFlags = getAccessedFeatureFlags()
+    currentProvider = provider
+    return accessedFeatureFlags
+  }
+
+  internal fun getAccessedFeatureFlags(): String? {
+    if (accessedFeatureFlags.isEmpty()) {
+      return null
+    }
+
+    return accessedFeatureFlags.joinToString(separator = ", ") { it }
   }
 }

packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.cpp

@@ -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<<0bb021affcef5162b578ad3f1e45508f>>
+ * @generated SignedSource<<f4dba9f073137759e18fb71411232d7a>>
  */
 
 /**
@@ -594,11 +594,25 @@ void JReactNativeFeatureFlagsCxxInterop::dangerouslyReset(
   ReactNativeFeatureFlags::dangerouslyReset();
 }
 
+jni::local_ref<jstring> JReactNativeFeatureFlagsCxxInterop::dangerouslyForceOverride(
+    facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/,
+    jni::alias_ref<jobject> provider) {
+  auto accessedFlags = ReactNativeFeatureFlags::dangerouslyForceOverride(
+             std::make_unique<ReactNativeFeatureFlagsProviderHolder>(provider));
+  if (accessedFlags.has_value()) {
+    return jni::make_jstring(accessedFlags.value());
+  }
+  return nullptr;
+}
+
 void JReactNativeFeatureFlagsCxxInterop::registerNatives() {
   javaClassLocal()->registerNatives({
       makeNativeMethod(
           "override", JReactNativeFeatureFlagsCxxInterop::override),
       makeNativeMethod("dangerouslyReset", JReactNativeFeatureFlagsCxxInterop::dangerouslyReset),
+      makeNativeMethod(
+          "dangerouslyForceOverride",
+          JReactNativeFeatureFlagsCxxInterop::dangerouslyForceOverride),
       makeNativeMethod(
         "commonTestFlag",
         JReactNativeFeatureFlagsCxxInterop::commonTestFlag),

packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.h

@@ -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<<fe97369e30d5359ffcd9c8e304dbc121>>
+ * @generated SignedSource<<9b325f01aea83bc93db31c9a63bea3f7>>
  */
 
 /**
@@ -184,6 +184,10 @@ class JReactNativeFeatureFlagsCxxInterop
   static void dangerouslyReset(
       facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);
 
+  static jni::local_ref<jstring> dangerouslyForceOverride(
+      facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>,
+      jni::alias_ref<jobject> provider);
+
   static void registerNatives();
 };
 

packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.cpp

@@ -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<<013c1fa01cf029635c04b50f83cc80ef>>
+ * @generated SignedSource<<14957b53f50e3ea943f0e3631475f336>>
  */
 
 /**
@@ -21,6 +21,11 @@
 
 namespace facebook::react {
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wglobal-constructors"
+std::unique_ptr<ReactNativeFeatureFlagsAccessor> accessor_;
+#pragma GCC diagnostic pop
+
 bool ReactNativeFeatureFlags::commonTestFlag() {
   return getAccessor().commonTestFlag();
 }
@@ -223,16 +228,26 @@ void ReactNativeFeatureFlags::override(
 }
 
 void ReactNativeFeatureFlags::dangerouslyReset() {
-  getAccessor(true);
+  accessor_ = std::make_unique<ReactNativeFeatureFlagsAccessor>();
+}
+
+std::optional<std::string> ReactNativeFeatureFlags::dangerouslyForceOverride(
+    std::unique_ptr<ReactNativeFeatureFlagsProvider> provider) {
+  auto accessor = std::make_unique<ReactNativeFeatureFlagsAccessor>();
+  accessor->override(std::move(provider));
+
+  std::swap(accessor_, accessor);
+
+  // Now accessor is the old accessor
+  return accessor == nullptr ? std::nullopt
+                             : accessor->getAccessedFeatureFlagNames();
 }
 
-ReactNativeFeatureFlagsAccessor& ReactNativeFeatureFlags::getAccessor(
-    bool reset) {
-  static std::unique_ptr<ReactNativeFeatureFlagsAccessor> accessor;
-  if (accessor == nullptr || reset) {
-    accessor = std::make_unique<ReactNativeFeatureFlagsAccessor>();
+ReactNativeFeatureFlagsAccessor& ReactNativeFeatureFlags::getAccessor() {
+  if (accessor_ == nullptr) {
+    accessor_ = std::make_unique<ReactNativeFeatureFlagsAccessor>();
   }
-  return *accessor;
+  return *accessor_;
 }
 
 } // namespace facebook::react

packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlags.h

@@ -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<<3bbe3eb333030be7f32c0965c9db4a5c>>
+ * @generated SignedSource<<c5a4993e3c7093f221cce2c891d86905>>
  */
 
 /**
@@ -22,6 +22,8 @@
 #include <react/featureflags/ReactNativeFeatureFlagsAccessor.h>
 #include <react/featureflags/ReactNativeFeatureFlagsProvider.h>
 #include <memory>
+#include <optional>
+#include <string>
 
 #ifndef RN_EXPORT
 #define RN_EXPORT __attribute__((visibility("default")))
@@ -317,9 +319,24 @@ class ReactNativeFeatureFlags {
    */
   RN_EXPORT static void dangerouslyReset();
 
+  /**
+   * This is a combination of `dangerouslyReset` and `override` that reduces
+   * the likeliness of a race condition between the two calls.
+   *
+   * This is **dangerous** because it can introduce consistency issues that will
+   * be much harder to debug. For example, it could hide the fact that feature
+   * flags are read before you set the values you want to use everywhere. It
+   * could also cause a workflow to suddently have different feature flags for
+   * behaviors that were configured with different values before.
+   *
+   * Please see the documentation of `dangerouslyReset` for additional details.
+   */
+  RN_EXPORT static std::optional<std::string> dangerouslyForceOverride(
+      std::unique_ptr<ReactNativeFeatureFlagsProvider> provider);
+
  private:
   ReactNativeFeatureFlags() = delete;
-  static ReactNativeFeatureFlagsAccessor& getAccessor(bool reset = false);
+  static ReactNativeFeatureFlagsAccessor& getAccessor();
 };
 
 } // namespace facebook::react

packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.cpp

@@ -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<<087de432d750fea45c0c564e626ce1d9>>
+ * @generated SignedSource<<c106a754b3abef1e9e4b503772f85b0f>>
  */
 
 /**
@@ -923,13 +923,8 @@ void ReactNativeFeatureFlagsAccessor::override(
   currentProvider_ = std::move(provider);
 }
 
-void ReactNativeFeatureFlagsAccessor::markFlagAsAccessed(
-    int position,
-    const char* flagName) {
-  accessedFeatureFlags_[position] = flagName;
-}
-
-void ReactNativeFeatureFlagsAccessor::ensureFlagsNotAccessed() {
+std::optional<std::string>
+ReactNativeFeatureFlagsAccessor::getAccessedFeatureFlagNames() const {
   std::ostringstream featureFlagListBuilder;
   for (const auto& featureFlagName : accessedFeatureFlags_) {
     if (featureFlagName != nullptr) {
@@ -943,10 +938,24 @@ void ReactNativeFeatureFlagsAccessor::ensureFlagsNotAccessed() {
         accessedFeatureFlagNames.substr(0, accessedFeatureFlagNames.size() - 2);
   }
 
-  if (!accessedFeatureFlagNames.empty()) {
+  return accessedFeatureFlagNames.empty()
+      ? std::nullopt
+      : std::optional{accessedFeatureFlagNames};
+}
+
+void ReactNativeFeatureFlagsAccessor::markFlagAsAccessed(
+    int position,
+    const char* flagName) {
+  accessedFeatureFlags_[position] = flagName;
+}
+
+void ReactNativeFeatureFlagsAccessor::ensureFlagsNotAccessed() {
+  auto accessedFeatureFlagNames = getAccessedFeatureFlagNames();
+
+  if (accessedFeatureFlagNames.has_value()) {
     throw std::runtime_error(
         "Feature flags were accessed before being overridden: " +
-        accessedFeatureFlagNames);
+        accessedFeatureFlagNames.value());
   }
 }
 

packages/react-native/ReactCommon/react/featureflags/ReactNativeFeatureFlagsAccessor.h

@@ -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<<447de2c56cd16e313210f14d0ee3e9e9>>
+ * @generated SignedSource<<0c9cfdad6a33dd4044a5945befb45522>>
  */
 
 /**
@@ -24,6 +24,7 @@
 #include <atomic>
 #include <memory>
 #include <optional>
+#include <string>
 
 namespace facebook::react {
 
@@ -82,6 +83,7 @@ class ReactNativeFeatureFlagsAccessor {
   bool useTurboModules();
 
   void override(std::unique_ptr<ReactNativeFeatureFlagsProvider> provider);
+  std::optional<std::string> getAccessedFeatureFlagNames() const;
 
  private:
   void markFlagAsAccessed(int position, const char* flagName);

packages/react-native/ReactCommon/react/featureflags/tests/ReactNativeFeatureFlagsTest.cpp

@@ -121,4 +121,27 @@ TEST_F(ReactNativeFeatureFlagsTest, allowsOverridingAgainAfterReset) {
   EXPECT_EQ(ReactNativeFeatureFlags::commonTestFlag(), true);
 }
 
+TEST_F(
+    ReactNativeFeatureFlagsTest,
+    allowsDangerouslyForcingOverridesWhenValuesHaveNotBeenAccessed) {
+  auto accessedFlags = ReactNativeFeatureFlags::dangerouslyForceOverride(
+      std::make_unique<ReactNativeFeatureFlagsTestOverrides>());
+
+  EXPECT_EQ(ReactNativeFeatureFlags::commonTestFlag(), true);
+  EXPECT_EQ(accessedFlags.has_value(), false);
+}
+
+TEST_F(
+    ReactNativeFeatureFlagsTest,
+    allowsDangerouslyForcingOverridesWhenValuesHaveBeenAccessed) {
+  EXPECT_EQ(ReactNativeFeatureFlags::commonTestFlag(), false);
+
+  auto accessedFlags = ReactNativeFeatureFlags::dangerouslyForceOverride(
+      std::make_unique<ReactNativeFeatureFlagsTestOverrides>());
+
+  EXPECT_EQ(ReactNativeFeatureFlags::commonTestFlag(), true);
+  EXPECT_EQ(accessedFlags.has_value(), true);
+  EXPECT_EQ(accessedFlags.value(), "commonTestFlag");
+}
+
 } // namespace facebook::react

packages/react-native/scripts/featureflags/templates/android/JReactNativeFeatureFlagsCxxInterop.cpp-template.js

@@ -88,11 +88,25 @@ void JReactNativeFeatureFlagsCxxInterop::dangerouslyReset(
   ReactNativeFeatureFlags::dangerouslyReset();
 }
 
+jni::local_ref<jstring> JReactNativeFeatureFlagsCxxInterop::dangerouslyForceOverride(
+    facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/,
+    jni::alias_ref<jobject> provider) {
+  auto accessedFlags = ReactNativeFeatureFlags::dangerouslyForceOverride(
+             std::make_unique<ReactNativeFeatureFlagsProviderHolder>(provider));
+  if (accessedFlags.has_value()) {
+    return jni::make_jstring(accessedFlags.value());
+  }
+  return nullptr;
+}
+
 void JReactNativeFeatureFlagsCxxInterop::registerNatives() {
   javaClassLocal()->registerNatives({
       makeNativeMethod(
           "override", JReactNativeFeatureFlagsCxxInterop::override),
       makeNativeMethod("dangerouslyReset", JReactNativeFeatureFlagsCxxInterop::dangerouslyReset),
+      makeNativeMethod(
+          "dangerouslyForceOverride",
+          JReactNativeFeatureFlagsCxxInterop::dangerouslyForceOverride),
 ${Object.entries(definitions.common)
   .map(
     ([flagName, flagConfig]) =>