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

Summary: X-link: facebook/litho#954 X-link: facebook/react-native#39132 Pull Request resolved: #1347 # Context Reviewed By: NickGerleman, astreet Differential Revision: D48607502 fbshipit-source-id: 79552bc76879d1fc15341423ae6fbadeab2fb7af
facebook · Aug 24, 2023 · 49fbd40 · 49fbd40
1 parent 38ad93c
commit 49fbd40
Show file tree

Hide file tree

Showing 6 changed files with 18 additions and 34 deletions.
diff --git a/enums.py b/enums.py
@@ -52,8 +52,6 @@
         "WebFlexBasis",
         # Conformance fix: https://github.com/facebook/yoga/pull/1028
         "AbsolutePercentageAgainstPaddingEdge",
-        # fix JNI local ref overflows
-        "FixJNILocalRefOverflows",
     ],
     "PrintOptions": [
         ("Layout", 1 << 0),

diff --git a/java/com/facebook/yoga/YogaExperimentalFeature.java b/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/java/jni/YGJNIVanilla.cpp b/java/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/javascript/src/generated/YGEnums.ts b/javascript/src/generated/YGEnums.ts
@@ -56,7 +56,6 @@ export enum Errata {
 export enum ExperimentalFeature {
   WebFlexBasis = 0,
   AbsolutePercentageAgainstPaddingEdge = 1,
-  FixJNILocalRefOverflows = 2,
 }
 
 export enum FlexDirection {
@@ -163,7 +162,6 @@ const constants = {
   ERRATA_CLASSIC: Errata.Classic,
   EXPERIMENTAL_FEATURE_WEB_FLEX_BASIS: ExperimentalFeature.WebFlexBasis,
   EXPERIMENTAL_FEATURE_ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE: ExperimentalFeature.AbsolutePercentageAgainstPaddingEdge,
-  EXPERIMENTAL_FEATURE_FIX_JNILOCAL_REF_OVERFLOWS: ExperimentalFeature.FixJNILocalRefOverflows,
   FLEX_DIRECTION_COLUMN: FlexDirection.Column,
   FLEX_DIRECTION_COLUMN_REVERSE: FlexDirection.ColumnReverse,
   FLEX_DIRECTION_ROW: FlexDirection.Row,

diff --git a/yoga/YGEnums.cpp b/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/yoga/YGEnums.h b/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,