8194238: Trying exceptions in MethodHandles
authorpsandoz
Wed, 24 Jan 2018 16:44:31 -0800
changeset 49790 403e2f61f384
parent 49789 27b359322b1e
child 49791 a0ac3c9b76dc
8194238: Trying exceptions in MethodHandles Reviewed-by: jrose, vlivanov, ahgross
src/java.base/share/classes/java/lang/invoke/MethodHandles.java
test/jdk/java/lang/invoke/TryFinallyTest.java
--- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java	Tue Jan 23 11:18:11 2018 -0500
+++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java	Wed Jan 24 16:44:31 2018 -0800
@@ -5893,6 +5893,19 @@
      * In rare cases where exceptions must be converted in that way, first wrap
      * the target with {@link #catchException(MethodHandle, Class, MethodHandle)}
      * to capture an outgoing exception, and then wrap with {@code tryFinally}.
+     * <p>
+     * It is recommended that the first parameter type of {@code cleanup} be
+     * declared {@code Throwable} rather than a narrower subtype.  This ensures
+     * {@code cleanup} will always be invoked with whatever exception that
+     * {@code target} throws.  Declaring a narrower type may result in a
+     * {@code ClassCastException} being thrown by the {@code try-finally}
+     * handle if the type of the exception thrown by {@code target} is not
+     * assignable to the first parameter type of {@code cleanup}.  Note that
+     * various exception types of {@code VirtualMachineError},
+     * {@code LinkageError}, and {@code RuntimeException} can in principle be
+     * thrown by almost any kind of Java code, and a finally clause that
+     * catches (say) only {@code IOException} would mask any of the others
+     * behind a {@code ClassCastException}.
      *
      * @param target the handle whose execution is to be wrapped in a {@code try} block.
      * @param cleanup the handle that is invoked in the finally block.
@@ -5909,7 +5922,6 @@
      */
     public static MethodHandle tryFinally(MethodHandle target, MethodHandle cleanup) {
         List<Class<?>> targetParamTypes = target.type().parameterList();
-        List<Class<?>> cleanupParamTypes = cleanup.type().parameterList();
         Class<?> rtype = target.type().returnType();
 
         tryFinallyChecks(target, cleanup);
@@ -5919,6 +5931,10 @@
         // target parameter list.
         cleanup = dropArgumentsToMatch(cleanup, (rtype == void.class ? 1 : 2), targetParamTypes, 0);
 
+        // Ensure that the intrinsic type checks the instance thrown by the
+        // target against the first parameter of cleanup
+        cleanup = cleanup.asType(cleanup.type().changeParameterType(0, Throwable.class));
+
         // Use asFixedArity() to avoid unnecessary boxing of last argument for VarargsCollector case.
         return MethodHandleImpl.makeTryFinally(target.asFixedArity(), cleanup.asFixedArity(), rtype, targetParamTypes);
     }
--- a/test/jdk/java/lang/invoke/TryFinallyTest.java	Tue Jan 23 11:18:11 2018 -0500
+++ b/test/jdk/java/lang/invoke/TryFinallyTest.java	Wed Jan 24 16:44:31 2018 -0800
@@ -24,9 +24,7 @@
  */
 
 /* @test
- * @bug 8139885
- * @bug 8150824
- * @bug 8150825
+ * @bug 8139885 8150824 8150825 8194238
  * @run testng/othervm -ea -esa test.java.lang.invoke.TryFinallyTest
  */
 
@@ -126,6 +124,19 @@
         assertTrue(caught);
     }
 
+    @Test
+    public static void testTryFinallyThrowableCheck() {
+        MethodHandle mh = MethodHandles.tryFinally(TryFinally.MH_throwingTarget,
+                                                   TryFinally.MH_catchingCleanup);
+        try {
+            mh.invoke();
+            fail("ClassCastException expected");
+        } catch (Throwable t) {
+            assertTrue("Throwable not assignable to ClassCastException: " + t,
+                       ClassCastException.class.isAssignableFrom(t.getClass()));
+        }
+    }
+
     static class TryFinally {
 
         static String greet(String whom) {
@@ -156,6 +167,17 @@
 
         static void voidCleanup(Throwable t) {}
 
+        static class T1 extends Throwable {}
+
+        static class T2 extends Throwable {}
+
+        static void throwingTarget() throws Throwable {
+            throw new T1();
+        }
+
+        static void catchingCleanup(T2 t) throws Throwable {
+        }
+
         static final Class<TryFinally> TRY_FINALLY = TryFinally.class;
 
         static final MethodType MT_greet = methodType(String.class, String.class);
@@ -166,6 +188,8 @@
         static final MethodType MT_exclaimMore = methodType(String.class, Throwable.class, String.class, String.class);
         static final MethodType MT_voidTarget = methodType(void.class);
         static final MethodType MT_voidCleanup = methodType(void.class, Throwable.class);
+        static final MethodType MT_throwingTarget = methodType(void.class);
+        static final MethodType MT_catchingCleanup = methodType(void.class, T2.class);
 
         static final MethodHandle MH_greet;
         static final MethodHandle MH_exclaim;
@@ -175,6 +199,8 @@
         static final MethodHandle MH_exclaimMore;
         static final MethodHandle MH_voidTarget;
         static final MethodHandle MH_voidCleanup;
+        static final MethodHandle MH_throwingTarget;
+        static final MethodHandle MH_catchingCleanup;
 
         static final MethodHandle MH_dummyTarget;
 
@@ -192,6 +218,8 @@
                 MH_exclaimMore = LOOKUP.findStatic(TRY_FINALLY, "exclaimMore", MT_exclaimMore);
                 MH_voidTarget = LOOKUP.findStatic(TRY_FINALLY, "voidTarget", MT_voidTarget);
                 MH_voidCleanup = LOOKUP.findStatic(TRY_FINALLY, "voidCleanup", MT_voidCleanup);
+                MH_throwingTarget = LOOKUP.findStatic(TRY_FINALLY, "throwingTarget", MT_throwingTarget);
+                MH_catchingCleanup = LOOKUP.findStatic(TRY_FINALLY, "catchingCleanup", MT_catchingCleanup);
                 MH_dummyTarget = MethodHandles.dropArguments(MH_voidTarget, 0, int.class, long.class, Object.class,
                         int.class, long.class, Object.class);
             } catch (Exception e) {