8068889: Calling a @FunctionalInterface from JS leaks internal objects

Reviewed-by: jlaskey, sundar

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/linker/Bootstrap.java

@@ -189,7 +189,7 @@ public final class Bootstrap {
      * @return true if the obj is an instance of @FunctionalInterface interface
      */
     public static boolean isFunctionalInterfaceObject(final Object obj) {
-        return !JSType.isPrimitive(obj) && (NashornBottomLinker.getFunctionalInterfaceMethod(obj.getClass()) != null);
+        return !JSType.isPrimitive(obj) && (NashornBeansLinker.getFunctionalInterfaceMethod(obj.getClass()) != null);
     }
 
     /**

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/linker/NashornBeansLinker.java

@@ -25,20 +25,29 @@
 
 package jdk.nashorn.internal.runtime.linker;
 
+import static jdk.nashorn.internal.lookup.Lookup.MH;
+import static jdk.nashorn.internal.runtime.ECMAErrors.typeError;
+
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import jdk.internal.dynalink.CallSiteDescriptor;
 import jdk.internal.dynalink.beans.BeansLinker;
 import jdk.internal.dynalink.linker.ConversionComparator.Comparison;
 import jdk.internal.dynalink.linker.GuardedInvocation;
 import jdk.internal.dynalink.linker.GuardingDynamicLinker;
 import jdk.internal.dynalink.linker.LinkRequest;
 import jdk.internal.dynalink.linker.LinkerServices;
+import jdk.internal.dynalink.support.Guards;
 import jdk.internal.dynalink.support.Lookup;
 import jdk.nashorn.api.scripting.ScriptUtils;
 import jdk.nashorn.internal.objects.NativeArray;
 import jdk.nashorn.internal.runtime.ConsString;
+import jdk.nashorn.internal.runtime.Context;
 import jdk.nashorn.internal.runtime.ScriptObject;
+import jdk.nashorn.internal.runtime.ScriptRuntime;
 import jdk.nashorn.internal.runtime.options.Options;
 
 /**
@@ -68,19 +77,49 @@ public class NashornBeansLinker implements GuardingDynamicLinker {
         FILTER_CONSSTRING    = lookup.findOwnStatic("consStringFilter", Object.class, Object.class);
     }
 
+    // cache of @FunctionalInterface method of implementor classes
+    private static final ClassValue<Method> FUNCTIONAL_IFACE_METHOD = new ClassValue<Method>() {
+        @Override
+        protected Method computeValue(final Class<?> type) {
+            return findFunctionalInterfaceMethod(type);
+        }
+    };
+
     private final BeansLinker beansLinker = new BeansLinker();
 
     @Override
     public GuardedInvocation getGuardedInvocation(final LinkRequest linkRequest, final LinkerServices linkerServices) throws Exception {
-        if (linkRequest.getReceiver() instanceof ConsString) {
+        final Object self = linkRequest.getReceiver();
+        final CallSiteDescriptor desc = linkRequest.getCallSiteDescriptor();
+        if (self instanceof ConsString) {
             // In order to treat ConsString like a java.lang.String we need a link request with a string receiver.
             final Object[] arguments = linkRequest.getArguments();
             arguments[0] = "";
-            final LinkRequest forgedLinkRequest = linkRequest.replaceArguments(linkRequest.getCallSiteDescriptor(), arguments);
+            final LinkRequest forgedLinkRequest = linkRequest.replaceArguments(desc, arguments);
             final GuardedInvocation invocation = getGuardedInvocation(beansLinker, forgedLinkRequest, linkerServices);
             // If an invocation is found we add a filter that makes it work for both Strings and ConsStrings.
             return invocation == null ? null : invocation.filterArguments(0, FILTER_CONSSTRING);
         }
+
+        if ("call".equals(desc.getNameToken(CallSiteDescriptor.OPERATOR))) {
+            // Support dyn:call on any object that supports some @FunctionalInterface
+            // annotated interface. This way Java method, constructor references or
+            // implementations of java.util.function.* interfaces can be called as though
+            // those are script functions.
+            final Method m = getFunctionalInterfaceMethod(self.getClass());
+            if (m != null) {
+                final MethodType callType = desc.getMethodType();
+                // 'callee' and 'thiz' passed from script + actual arguments
+                if (callType.parameterCount() != m.getParameterCount() + 2) {
+                    throw typeError("no.method.matches.args", ScriptRuntime.safeToString(self));
+                }
+                return new GuardedInvocation(
+                        // drop 'thiz' passed from the script.
+                        MH.dropArguments(desc.getLookup().unreflect(m), 1, callType.parameterType(1)),
+                        Guards.getInstanceOfGuard(m.getDeclaringClass())).asTypeSafeReturn(
+                                new NashornBeansLinkerServices(linkerServices), callType);
+            }
+        }
         return getGuardedInvocation(beansLinker, linkRequest, linkerServices);
     }
 
@@ -137,6 +176,38 @@ public class NashornBeansLinker implements GuardingDynamicLinker {
         return arg instanceof ConsString ? arg.toString() : arg;
     }
 
+    private static Method findFunctionalInterfaceMethod(final Class<?> clazz) {
+        if (clazz == null) {
+            return null;
+        }
+
+        for (final Class<?> iface : clazz.getInterfaces()) {
+            // check accessiblity up-front
+            if (! Context.isAccessibleClass(iface)) {
+                continue;
+            }
+
+            // check for @FunctionalInterface
+            if (iface.isAnnotationPresent(FunctionalInterface.class)) {
+                // return the first abstract method
+                for (final Method m : iface.getMethods()) {
+                    if (Modifier.isAbstract(m.getModifiers())) {
+                        return m;
+                    }
+                }
+            }
+        }
+
+        // did not find here, try super class
+        return findFunctionalInterfaceMethod(clazz.getSuperclass());
+    }
+
+    // Returns @FunctionalInterface annotated interface's single abstract
+    // method. If not found, returns null.
+    static Method getFunctionalInterfaceMethod(final Class<?> clazz) {
+        return FUNCTIONAL_IFACE_METHOD.get(clazz);
+    }
+
     private static class NashornBeansLinkerServices implements LinkerServices {
         private final LinkerServices linkerServices;
 

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/linker/NashornBottomLinker.java

@@ -30,9 +30,6 @@ import static jdk.nashorn.internal.runtime.ECMAErrors.typeError;
 import static jdk.nashorn.internal.runtime.ScriptRuntime.UNDEFINED;
 
 import java.lang.invoke.MethodHandle;
-import java.lang.invoke.MethodType;
-import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
 import java.util.HashMap;
 import java.util.Map;
 import jdk.internal.dynalink.CallSiteDescriptor;
@@ -45,7 +42,6 @@ import jdk.internal.dynalink.linker.LinkRequest;
 import jdk.internal.dynalink.linker.LinkerServices;
 import jdk.internal.dynalink.support.Guards;
 import jdk.nashorn.internal.codegen.types.Type;
-import jdk.nashorn.internal.runtime.Context;
 import jdk.nashorn.internal.runtime.JSType;
 import jdk.nashorn.internal.runtime.ScriptRuntime;
 import jdk.nashorn.internal.runtime.UnwarrantedOptimismException;
@@ -95,22 +91,6 @@ final class NashornBottomLinker implements GuardingDynamicLinker, GuardingTypeCo
             }
             throw typeError("not.a.function", ScriptRuntime.safeToString(self));
         case "call":
-            // Support dyn:call on any object that supports some @FunctionalInterface
-            // annotated interface. This way Java method, constructor references or
-            // implementations of java.util.function.* interfaces can be called as though
-            // those are script functions.
-            final Method m = getFunctionalInterfaceMethod(self.getClass());
-            if (m != null) {
-                final MethodType callType = desc.getMethodType();
-                // 'callee' and 'thiz' passed from script + actual arguments
-                if (callType.parameterCount() != m.getParameterCount() + 2) {
-                    throw typeError("no.method.matches.args", ScriptRuntime.safeToString(self));
-                }
-                return Bootstrap.asTypeSafeReturn(new GuardedInvocation(
-                        // drop 'thiz' passed from the script.
-                        MH.dropArguments(desc.getLookup().unreflect(m), 1, callType.parameterType(1)),
-                        Guards.getInstanceOfGuard(m.getDeclaringClass())), linkerServices, desc);
-            }
             if(BeansLinker.isDynamicConstructor(self)) {
                 throw typeError("constructor.requires.new", ScriptRuntime.safeToString(self));
             }
@@ -218,44 +198,4 @@ final class NashornBottomLinker implements GuardingDynamicLinker, GuardingTypeCo
         }
         return ScriptRuntime.safeToString(linkRequest.getArguments()[1]);
     }
-
-    // cache of @FunctionalInterface method of implementor classes
-    private static final ClassValue<Method> FUNCTIONAL_IFACE_METHOD = new ClassValue<Method>() {
-        @Override
-        protected Method computeValue(final Class<?> type) {
-            return findFunctionalInterfaceMethod(type);
-        }
-
-        private Method findFunctionalInterfaceMethod(final Class<?> clazz) {
-            if (clazz == null) {
-                return null;
-            }
-
-            for (final Class<?> iface : clazz.getInterfaces()) {
-                // check accessiblity up-front
-                if (! Context.isAccessibleClass(iface)) {
-                    continue;
-                }
-
-                // check for @FunctionalInterface
-                if (iface.isAnnotationPresent(FunctionalInterface.class)) {
-                    // return the first abstract method
-                    for (final Method m : iface.getMethods()) {
-                        if (Modifier.isAbstract(m.getModifiers())) {
-                            return m;
-                        }
-                    }
-                }
-            }
-
-            // did not find here, try super class
-            return findFunctionalInterfaceMethod(clazz.getSuperclass());
-        }
-    };
-
-    // Returns @FunctionalInterface annotated interface's single abstract
-    // method. If not found, returns null.
-    static Method getFunctionalInterfaceMethod(final Class<?> clazz) {
-        return FUNCTIONAL_IFACE_METHOD.get(clazz);
-    }
 }

nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java

@@ -30,12 +30,16 @@ import static org.testng.Assert.assertNotNull;
 import static org.testng.Assert.assertNull;
 import static org.testng.Assert.assertTrue;
 import static org.testng.Assert.fail;
+
 import java.io.StringReader;
 import java.io.StringWriter;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
 import java.lang.reflect.Proxy;
 import java.util.concurrent.Callable;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Consumer;
+import java.util.function.Function;
 import javax.script.Compilable;
 import javax.script.CompiledScript;
 import javax.script.Invocable;
@@ -680,6 +684,41 @@ public class ScriptEngineTest {
         assertNull(value);
     }
 
+    // @bug JDK-8068889: ConsString arguments to a functional interface wasn't converted to string.
+    @Test
+    public void functionalInterfaceStringTest() throws Exception {
+        final ScriptEngineManager manager = new ScriptEngineManager();
+        final ScriptEngine e = manager.getEngineByName("nashorn");
+        final AtomicBoolean invoked = new AtomicBoolean(false);
+        e.put("f", new Function<String, String>() {
+            @Override
+            public String apply(String t) {
+                invoked.set(true);
+                return t;
+            }
+        });
+        assertEquals(e.eval("var x = 'a'; x += 'b'; f(x)"), "ab");
+        assertTrue(invoked.get());
+    }
+
+    // @bug JDK-8068889: ScriptObject arguments to a functional interface wasn't converted to a mirror.
+    @Test
+    public void functionalInterfaceObjectTest() throws Exception {
+        final ScriptEngineManager manager = new ScriptEngineManager();
+        final ScriptEngine e = manager.getEngineByName("nashorn");
+        final AtomicBoolean invoked = new AtomicBoolean(false);
+        e.put("c", new Consumer<Object>() {
+            @Override
+            public void accept(Object t) {
+                assertTrue(t instanceof ScriptObjectMirror);
+                assertEquals(((ScriptObjectMirror)t).get("a"), "xyz");
+                invoked.set(true);
+            }
+        });
+        e.eval("var x = 'xy'; x += 'z';c({a:x})");
+        assertTrue(invoked.get());
+    }
+
     private static void checkProperty(final ScriptEngine e, final String name)
         throws ScriptException {
         final String value = System.getProperty(name);