From 688bd4ef729306123baf5cae6987b881460c88f6 Mon Sep 17 00:00:00 2001 From: Sidharth Guglani Date: Tue, 22 Oct 2019 10:45:20 -0700 Subject: [PATCH] Add exception handling in vanilla jni Summary: Exception handling in vanilla jni ## Changelog: [Internal] [Added] Added exception handling for vanilla jni implementation in yoga Reviewed By: amir-shalem Differential Revision: D18036134 fbshipit-source-id: 965eaa2fddbc00b9ac0120b79678608e280d03db --- java/jni/YGJNIVanilla.cpp | 38 ++-- java/jni/corefunctions.cpp | 14 +- .../com/facebook/yoga/YogaExceptionTest.java | 165 ++++++++++++++++++ 3 files changed, 195 insertions(+), 22 deletions(-) create mode 100644 java/tests/com/facebook/yoga/YogaExceptionTest.java diff --git a/java/jni/YGJNIVanilla.cpp b/java/jni/YGJNIVanilla.cpp index 9835758f..9a38c575 100644 --- a/java/jni/YGJNIVanilla.cpp +++ b/java/jni/YGJNIVanilla.cpp @@ -352,25 +352,29 @@ static void jni_YGNodeCalculateLayoutJNI( jlongArray nativePointers, jobjectArray javaNodes) { - void* layoutContext = nullptr; - auto map = PtrJNodeMapVanilla{}; - if (nativePointers) { - size_t nativePointersSize = env->GetArrayLength(nativePointers); - jlong result[nativePointersSize]; - env->GetLongArrayRegion(nativePointers, 0, nativePointersSize, result); + try { + void* layoutContext = nullptr; + auto map = PtrJNodeMapVanilla{}; + if (nativePointers) { + size_t nativePointersSize = env->GetArrayLength(nativePointers); + jlong result[nativePointersSize]; + env->GetLongArrayRegion(nativePointers, 0, nativePointersSize, result); - map = PtrJNodeMapVanilla{result, nativePointersSize, javaNodes}; - layoutContext = ↦ + map = PtrJNodeMapVanilla{result, nativePointersSize, javaNodes}; + layoutContext = ↦ + } + + const YGNodeRef root = _jlong2YGNodeRef(nativePointer); + YGNodeCalculateLayoutWithContext( + root, + static_cast(width), + static_cast(height), + YGNodeStyleGetDirection(_jlong2YGNodeRef(nativePointer)), + layoutContext); + YGTransferLayoutOutputsRecursive(env, obj, root, layoutContext); + } catch (jthrowable throwable) { + env->Throw(throwable); } - - const YGNodeRef root = _jlong2YGNodeRef(nativePointer); - YGNodeCalculateLayoutWithContext( - root, - static_cast(width), - static_cast(height), - YGNodeStyleGetDirection(_jlong2YGNodeRef(nativePointer)), - layoutContext); - YGTransferLayoutOutputsRecursive(env, obj, root, layoutContext); } static void jni_YGNodeMarkDirtyJNI( diff --git a/java/jni/corefunctions.cpp b/java/jni/corefunctions.cpp index 37dc12d4..6a250b1d 100644 --- a/java/jni/corefunctions.cpp +++ b/java/jni/corefunctions.cpp @@ -62,12 +62,16 @@ void logErrorMessageAndDie(const char* message) { } void assertNoPendingJniException(JNIEnv* env) { - // This method cannot call any other method of the library, since other - // methods of the library use it to check for exceptions too - if (env->ExceptionCheck()) { - env->ExceptionDescribe(); - logErrorMessageAndDie("Aborting due to pending Java exception in JNI"); + if (env->ExceptionCheck() == JNI_FALSE) { + return; } + + auto throwable = env->ExceptionOccurred(); + if (!throwable) { + logErrorMessageAndDie("Unable to get pending JNI exception."); + } + env->ExceptionClear(); + throw throwable; } } // namespace vanillajni diff --git a/java/tests/com/facebook/yoga/YogaExceptionTest.java b/java/tests/com/facebook/yoga/YogaExceptionTest.java new file mode 100644 index 00000000..c7b650f6 --- /dev/null +++ b/java/tests/com/facebook/yoga/YogaExceptionTest.java @@ -0,0 +1,165 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.yoga; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.lang.ref.WeakReference; +import java.util.ArrayList; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class YogaExceptionTest { + @Parameterized.Parameters(name = "{0}") + public static Iterable nodeFactories() { + return TestParametrization.nodeFactories(); + } + + @Parameterized.Parameter public TestParametrization.NodeFactory mNodeFactory; + + @Test(expected = RuntimeException.class) + public void testBaselineThrows() { + final YogaNode root = createNode(); + root.setFlexDirection(YogaFlexDirection.ROW); + root.setAlignItems(YogaAlign.BASELINE); + + final YogaNode child1 = createNode(); + root.addChildAt(child1, 0); + + final YogaNode child2 = createNode(); + child2.setBaselineFunction(new YogaBaselineFunction() { + public float baseline(YogaNode node, float width, float height) { + throw new RuntimeException(); + } + }); + root.addChildAt(child2, 1); + + root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED); + } + + @Test + public void testBaselineThrowsAndStops() { + final YogaNode root = createNode(); + root.setFlexDirection(YogaFlexDirection.ROW); + root.setAlignItems(YogaAlign.BASELINE); + + final YogaNode child1 = createNode(); + root.addChildAt(child1, 0); + + final YogaNode child2 = createNode(); + final AtomicReference expected = new AtomicReference(); + child2.setBaselineFunction(new YogaBaselineFunction() { + public float baseline(YogaNode node, float width, float height) { + RuntimeException e = new RuntimeException(); + expected.set(e); + throw e; + } + }); + root.addChildAt(child2, 1); + + final YogaNode child3 = createNode(); + final AtomicBoolean child3Called = new AtomicBoolean(); + child3.setBaselineFunction(new YogaBaselineFunction() { + public float baseline(YogaNode node, float width, float height) { + child3Called.set(true); + return 1.0f; + } + }); + root.addChildAt(child3, 2); + + try { + root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED); + fail(); + } catch (RuntimeException e) { + assertEquals(expected.get(), e); + } + assertFalse(child3Called.get()); + } + + @Test(expected = RuntimeException.class) + public void testMeasureThrows() { + final YogaNode node = createNode(); + node.setMeasureFunction(new YogaMeasureFunction() { + public long measure( + YogaNode node, + float width, + YogaMeasureMode widthMode, + float height, + YogaMeasureMode heightMode) { + throw new RuntimeException(); + } + }); + node.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED); + } + + @Test + public void testMeasureThrowsAndStops() { + final YogaNode root = createNode(); + root.setFlexDirection(YogaFlexDirection.ROW); + root.setAlignItems(YogaAlign.BASELINE); + + final YogaNode child1 = createNode(); + root.addChildAt(child1, 0); + + final YogaNode child2 = createNode(); + final AtomicReference expected = new AtomicReference(); + child2.setMeasureFunction(new YogaMeasureFunction() { + public long measure( + YogaNode node, + float width, + YogaMeasureMode widthMode, + float height, + YogaMeasureMode heightMode) { + RuntimeException e = new RuntimeException(); + expected.set(e); + throw e; + } + }); + root.addChildAt(child2, 1); + + final YogaNode child3 = createNode(); + final AtomicBoolean child3Called = new AtomicBoolean(); + child3.setMeasureFunction(new YogaMeasureFunction() { + public long measure( + YogaNode node, + float width, + YogaMeasureMode widthMode, + float height, + YogaMeasureMode heightMode) { + child3Called.set(true); + return 1; + } + }); + root.addChildAt(child3, 2); + + try { + root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED); + fail(); + } catch (RuntimeException e) { + assertEquals(expected.get(), e); + } + assertFalse(child3Called.get()); + } + + private YogaNode createNode() { + return mNodeFactory.create(); + } + + private YogaNode createNode(YogaConfig config) { + return mNodeFactory.create(config); + } +}