Ship the fix for local reference overflow in Yoga (facebook#954)

Summary: Pull Request resolved: facebook#954 X-link: facebook/react-native#39132 X-link: facebook/yoga#1347 # Context Reviewed By: NickGerleman, astreet Differential Revision: D48607502 fbshipit-source-id: 83601ba7c1f347ac0c2e141e1d59d1f6f01a1329
pentiumao · Aug 24, 2023 · 660c69d · 660c69d
1 parent b787204
commit 660c69d
Show file tree

Hide file tree

Showing 6 changed files with 19 additions and 41 deletions.
diff --git a/lib/yoga/src/main/cpp/yoga/YGEnums.cpp b/lib/yoga/src/main/cpp/yoga/YGEnums.cpp
@@ -107,8 +107,6 @@ const char* YGExperimentalFeatureToString(const YGExperimentalFeature value) {
       return "web-flex-basis";
     case YGExperimentalFeatureAbsolutePercentageAgainstPaddingEdge:
       return "absolute-percentage-against-padding-edge";
-    case YGExperimentalFeatureFixJNILocalRefOverflows:
-      return "fix-jnilocal-ref-overflows";
   }
   return "unknown";
 }

diff --git a/lib/yoga/src/main/cpp/yoga/YGEnums.h b/lib/yoga/src/main/cpp/yoga/YGEnums.h
@@ -65,8 +65,7 @@ YG_DEFINE_ENUM_FLAG_OPERATORS(YGErrata)
 YG_ENUM_SEQ_DECL(
     YGExperimentalFeature,
     YGExperimentalFeatureWebFlexBasis,
-    YGExperimentalFeatureAbsolutePercentageAgainstPaddingEdge,
-    YGExperimentalFeatureFixJNILocalRefOverflows)
+    YGExperimentalFeatureAbsolutePercentageAgainstPaddingEdge)
 
 YG_ENUM_SEQ_DECL(
     YGFlexDirection,

diff --git a/lib/yoga/src/main/java/com/facebook/yoga/YogaExperimentalFeature.java b/lib/yoga/src/main/java/com/facebook/yoga/YogaExperimentalFeature.java
@@ -11,8 +11,7 @@
 
 public enum YogaExperimentalFeature {
   WEB_FLEX_BASIS(0),
-  ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE(1),
-  FIX_JNILOCAL_REF_OVERFLOWS(2);
+  ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE(1);
 
   private final int mIntValue;
 
@@ -28,7 +27,6 @@ public static YogaExperimentalFeature fromInt(int value) {
     switch (value) {
       case 0: return WEB_FLEX_BASIS;
       case 1: return ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE;
-      case 2: return FIX_JNILOCAL_REF_OVERFLOWS;
       default: throw new IllegalArgumentException("Unknown enum value: " + value);
     }
   }

diff --git a/lib/yogajni/src/main/cpp/jni/YGJNIVanilla.cpp b/lib/yogajni/src/main/cpp/jni/YGJNIVanilla.cpp
@@ -282,8 +282,7 @@ static void YGTransferLayoutOutputsRecursive(
     JNIEnv* env,
     jobject thiz,
     YGNodeRef root,
-    void* layoutContext,
-    bool shouldCleanLocalRef) {
+    void* layoutContext) {
   if (!YGNodeGetHasNewLayout(root)) {
     return;
   }
@@ -337,28 +336,26 @@ static void YGTransferLayoutOutputsRecursive(
     arr[borderStartIndex + 3] = YGNodeLayoutGetBorder(root, YGEdgeBottom);
   }
 
-  // Don't change this field name without changing the name of the field in
-  // Database.java
-  auto objectClass = facebook::yoga::vanillajni::make_local_ref(
-      env, env->GetObjectClass(obj.get()));
-  static const jfieldID arrField = facebook::yoga::vanillajni::getFieldId(
-      env, objectClass.get(), "arr", "[F");
-
-  ScopedLocalRef<jfloatArray> arrFinal =
-      make_local_ref(env, env->NewFloatArray(arrSize));
-  env->SetFloatArrayRegion(arrFinal.get(), 0, arrSize, arr);
-  env->SetObjectField(obj.get(), arrField, arrFinal.get());
-
-  if (shouldCleanLocalRef) {
-    objectClass.reset();
-    arrFinal.reset();
+  // Create scope to make sure to release any local refs created here
+  {
+    // Don't change this field name without changing the name of the field in
+    // Database.java
+    auto objectClass = facebook::yoga::vanillajni::make_local_ref(
+        env, env->GetObjectClass(obj.get()));
+    static const jfieldID arrField = facebook::yoga::vanillajni::getFieldId(
+        env, objectClass.get(), "arr", "[F");
+
+    ScopedLocalRef<jfloatArray> arrFinal =
+        make_local_ref(env, env->NewFloatArray(arrSize));
+    env->SetFloatArrayRegion(arrFinal.get(), 0, arrSize, arr);
+    env->SetObjectField(obj.get(), arrField, arrFinal.get());
   }
 
   YGNodeSetHasNewLayout(root, false);
 
   for (uint32_t i = 0; i < YGNodeGetChildCount(root); i++) {
     YGTransferLayoutOutputsRecursive(
-        env, thiz, YGNodeGetChild(root, i), layoutContext, shouldCleanLocalRef);
+        env, thiz, YGNodeGetChild(root, i), layoutContext);
   }
 }
 
@@ -380,17 +377,13 @@ static void jni_YGNodeCalculateLayoutJNI(
     }
 
     const YGNodeRef root = _jlong2YGNodeRef(nativePointer);
-    const bool shouldCleanLocalRef =
-        root->getConfig()->isExperimentalFeatureEnabled(
-            YGExperimentalFeatureFixJNILocalRefOverflows);
     YGNodeCalculateLayoutWithContext(
         root,
         static_cast<float>(width),
         static_cast<float>(height),
         YGNodeStyleGetDirection(_jlong2YGNodeRef(nativePointer)),
         layoutContext);
-    YGTransferLayoutOutputsRecursive(
-        env, obj, root, layoutContext, shouldCleanLocalRef);
+    YGTransferLayoutOutputsRecursive(env, obj, root, layoutContext);
   } catch (const YogaJniException& jniException) {
     ScopedLocalRef<jthrowable> throwable = jniException.getThrowable();
     if (throwable.get()) {

diff --git a/litho-core/src/main/java/com/facebook/litho/NodeConfig.kt b/litho-core/src/main/java/com/facebook/litho/NodeConfig.kt
@@ -16,10 +16,8 @@
 
 package com.facebook.litho
 
-import com.facebook.litho.config.ComponentsConfiguration
 import com.facebook.litho.yoga.LithoYogaFactory
 import com.facebook.yoga.YogaConfig
-import com.facebook.yoga.YogaExperimentalFeature
 import com.facebook.yoga.YogaNode
 import kotlin.jvm.JvmField
 
@@ -33,13 +31,7 @@ object NodeConfig {
   @JvmField @Volatile var yogaNodeFactory: InternalYogaNodeFactory? = null
 
   /** Allows access to the internal YogaConfig instance */
-  @get:JvmStatic
-  val yogaConfig: YogaConfig =
-      LithoYogaFactory.createYogaConfig().apply {
-        if (ComponentsConfiguration.enableFixForJniLocalRefOverflow) {
-          setExperimentalFeatureEnabled(YogaExperimentalFeature.FIX_JNILOCAL_REF_OVERFLOWS, true)
-        }
-      }
+  @get:JvmStatic val yogaConfig: YogaConfig = LithoYogaFactory.createYogaConfig()
 
   @JvmStatic
   fun createYogaNode(): YogaNode {

diff --git a/litho-core/src/main/java/com/facebook/litho/config/ComponentsConfiguration.java b/litho-core/src/main/java/com/facebook/litho/config/ComponentsConfiguration.java
@@ -236,8 +236,6 @@ public static boolean isSplitResolveAndLayoutWithSplitHandlers() {
 
   public static boolean enableLayoutCaching = false;
 
-  public static boolean enableFixForJniLocalRefOverflow = false;
-
   public static int recyclerBinderStrategy = 0;
 
   public static boolean enableMountableRecycler = false;