From cf62383f7d9c7bedfcb0882640bf9d5cd318881a Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Tue, 16 Mar 2021 08:52:30 +0100 Subject: [PATCH 1/3] Handle duplicate types, handle too many interfaces. Don't cache error results for multiple types --- .../runtime/linker/AdaptationException.java | 6 ++- .../runtime/linker/AdaptationResult.java | 2 + .../linker/JavaAdapterBytecodeGenerator.java | 6 +-- .../runtime/linker/JavaAdapterFactory.java | 34 +++++++++----- .../runtime/resources/Messages.properties | 2 + test/nashorn/script/basic/duplicate-extend.js | 45 +++++++++++++++++++ .../script/basic/duplicate-extend.js.EXPECTED | 2 + 7 files changed, 82 insertions(+), 15 deletions(-) create mode 100644 test/nashorn/script/basic/duplicate-extend.js create mode 100644 test/nashorn/script/basic/duplicate-extend.js.EXPECTED diff --git a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/AdaptationException.java b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/AdaptationException.java index 4bad533e6..7d7baa76f 100644 --- a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/AdaptationException.java +++ b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/AdaptationException.java @@ -30,8 +30,12 @@ final class AdaptationException extends Exception { private final AdaptationResult adaptationResult; AdaptationException(final AdaptationResult.Outcome outcome, final String classList) { + this(new AdaptationResult(outcome, classList)); + } + + AdaptationException(final AdaptationResult result) { super(null, null, false, false); - this.adaptationResult = new AdaptationResult(outcome, classList); + this.adaptationResult = result; } AdaptationResult getAdaptationResult() { diff --git a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/AdaptationResult.java b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/AdaptationResult.java index 207e17abe..707f17fa5 100644 --- a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/AdaptationResult.java +++ b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/AdaptationResult.java @@ -47,6 +47,8 @@ enum Outcome { ERROR_NON_PUBLIC_CLASS, ERROR_NO_ACCESSIBLE_CONSTRUCTOR, ERROR_MULTIPLE_SUPERCLASSES, + ERROR_DUPLICATE_TYPE, + ERROR_TOO_MANY_INTERFACES, ERROR_NO_COMMON_LOADER, ERROR_FINAL_FINALIZER, ERROR_OTHER diff --git a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterBytecodeGenerator.java b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterBytecodeGenerator.java index fca56f3bd..07dbbc458 100644 --- a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterBytecodeGenerator.java +++ b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterBytecodeGenerator.java @@ -407,9 +407,9 @@ private boolean generateConstructors(final Constructor ctor) { return false; } - // Generate a constructor that delegates to ctor, but takes an additional ScriptObject parameter at the - // beginning of its parameter list. - generateOverridingConstructor(ctor, false); + // Generate a constructor that delegates to ctor, but takes an additional ScriptObject parameter at the + // beginning of its parameter list. + generateOverridingConstructor(ctor, false); if (samName == null) { return false; diff --git a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java index 28f517773..106034168 100644 --- a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java +++ b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java @@ -40,8 +40,10 @@ import java.security.PrivilegedAction; import java.security.ProtectionDomain; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import jdk.dynalink.CallSiteDescriptor; import jdk.dynalink.StandardOperation; @@ -223,21 +225,24 @@ private static AdapterInfo getAdapterInfo(final Class[] types) { private static AdapterInfo createAdapterInfo(final List> types, final ClassAndLoader definingClassAndLoader) { Class superClass = null; final List> interfaces = new ArrayList<>(types.size()); + final Set> interfacesDedup = new HashSet<>(Math.max((int) (types.size()/.75f) + 1, 16)); for(final Class t: types) { final int mod = t.getModifiers(); if(!t.isInterface()) { - if(superClass != null) { - return new AdapterInfo(AdaptationResult.Outcome.ERROR_MULTIPLE_SUPERCLASSES, t.getCanonicalName() + " and " + superClass.getCanonicalName()); - } - if (Modifier.isFinal(mod)) { + if (superClass == t) { + throw new AdaptationResult(Outcome.ERROR_DUPLICATE_TYPE, t.getCanonicalName()).typeError(); + } else if(superClass != null) { + throw new AdaptationResult(Outcome.ERROR_MULTIPLE_SUPERCLASSES, t.getCanonicalName() + " and " + superClass.getCanonicalName()).typeError(); + } else if (Modifier.isFinal(mod)) { return new AdapterInfo(AdaptationResult.Outcome.ERROR_FINAL_CLASS, t.getCanonicalName()); } superClass = t; } else { if (interfaces.size() > 65535) { - throw new IllegalArgumentException("interface limit exceeded"); + throw new AdaptationResult(Outcome.ERROR_TOO_MANY_INTERFACES, "65535").typeError(); + } else if (!interfacesDedup.add(t)) { + throw new AdaptationResult(Outcome.ERROR_DUPLICATE_TYPE, t.getCanonicalName()).typeError(); } - interfaces.add(t); } @@ -246,15 +251,22 @@ private static AdapterInfo createAdapterInfo(final List> types, final C } } - final Class effectiveSuperClass = superClass == null ? Object.class : superClass; return AccessController.doPrivileged((PrivilegedAction) () -> { try { - return new AdapterInfo(effectiveSuperClass, interfaces, definingClassAndLoader); + try { + return new AdapterInfo(effectiveSuperClass, interfaces, definingClassAndLoader); + } catch (final RuntimeException e) { + throw new AdaptationException(new AdaptationResult(Outcome.ERROR_OTHER, e, types.toString(), e.toString())); + } } catch (final AdaptationException e) { - return new AdapterInfo(e.getAdaptationResult()); - } catch (final RuntimeException e) { - return new AdapterInfo(new AdaptationResult(Outcome.ERROR_OTHER, e, types.toString(), e.toString())); + final var res = e.getAdaptationResult(); + if (types.size() > 1) { + // Only cache error outcomes for single types + throw res.typeError(); + } else { + return new AdapterInfo(res); + } } }, CREATE_ADAPTER_INFO_ACC_CTXT); } diff --git a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/resources/Messages.properties b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/resources/Messages.properties index 6dbdd869b..c355c4833 100644 --- a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/resources/Messages.properties +++ b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/resources/Messages.properties @@ -174,6 +174,8 @@ type.error.extend.ERROR_FINAL_CLASS=Can not extend final class {0}. type.error.extend.ERROR_NON_PUBLIC_CLASS=Can not extend/implement non-public class/interface {0}. type.error.extend.ERROR_NO_ACCESSIBLE_CONSTRUCTOR=Can not extend class {0} as it has no public or protected constructors. type.error.extend.ERROR_MULTIPLE_SUPERCLASSES=Can not extend multiple classes {0}. At most one of the specified types can be a class, the rest must all be interfaces. +type.error.extend.ERROR_DUPLICATE_TYPE=Type {0} specified multiple times. +type.error.extend.ERROR_TOO_MANY_INTERFACES=Too many interfaces specified. At most {0} are supported by the JVM. type.error.extend.ERROR_NO_COMMON_LOADER=Can not find a common class loader for ScriptObject and {0}. type.error.extend.ERROR_FINAL_FINALIZER=Can not extend class because {0} has a final finalize method. type.error.extend.ERROR_OTHER=Can not extend/implement {0} because of {1} diff --git a/test/nashorn/script/basic/duplicate-extend.js b/test/nashorn/script/basic/duplicate-extend.js new file mode 100644 index 000000000..e031a3ea6 --- /dev/null +++ b/test/nashorn/script/basic/duplicate-extend.js @@ -0,0 +1,45 @@ +/* + * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * Specifying duplicate interfaces or classes throws a TypeError. + * + * @test + * @run + */ + +var Consumer = Java.type("java.util.function.Consumer"); +var JFunction = Java.type("java.util.function.Function"); +var TimerTask = Java.type("java.util.TimerTask"); + +try { + Java.extend(Consumer,JFunction,Consumer) +} catch (e) { + print(e.message) +} + +try { + Java.extend(TimerTask,Consumer,TimerTask) +} catch (e) { + print(e.message) +} diff --git a/test/nashorn/script/basic/duplicate-extend.js.EXPECTED b/test/nashorn/script/basic/duplicate-extend.js.EXPECTED new file mode 100644 index 000000000..5d15baadf --- /dev/null +++ b/test/nashorn/script/basic/duplicate-extend.js.EXPECTED @@ -0,0 +1,2 @@ +Type java.util.function.Consumer specified multiple times. +Type java.util.TimerTask specified multiple times. From 9c52fc083953a95afdb734695cebc6c595aa6986 Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Sun, 21 Mar 2021 18:07:01 +0100 Subject: [PATCH 2/3] Simplify the adaptation result, it was overcomplicated due to caching that autoConvertibleFromFunction field. --- .../runtime/linker/AdaptationException.java | 44 ------- .../runtime/linker/AdaptationResult.java | 80 ------------ .../linker/JavaAdapterBytecodeGenerator.java | 17 ++- .../runtime/linker/JavaAdapterFactory.java | 114 ++++++++---------- .../runtime/resources/Messages.properties | 17 ++- 5 files changed, 66 insertions(+), 206 deletions(-) delete mode 100644 src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/AdaptationException.java delete mode 100644 src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/AdaptationResult.java diff --git a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/AdaptationException.java b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/AdaptationException.java deleted file mode 100644 index 7d7baa76f..000000000 --- a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/AdaptationException.java +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. Oracle designates this - * particular file as subject to the "Classpath" exception as provided - * by Oracle in the LICENSE file that accompanied this code. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - -package org.openjdk.nashorn.internal.runtime.linker; - -@SuppressWarnings("serial") -final class AdaptationException extends Exception { - private final AdaptationResult adaptationResult; - - AdaptationException(final AdaptationResult.Outcome outcome, final String classList) { - this(new AdaptationResult(outcome, classList)); - } - - AdaptationException(final AdaptationResult result) { - super(null, null, false, false); - this.adaptationResult = result; - } - - AdaptationResult getAdaptationResult() { - return adaptationResult; - } -} diff --git a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/AdaptationResult.java b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/AdaptationResult.java deleted file mode 100644 index 707f17fa5..000000000 --- a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/AdaptationResult.java +++ /dev/null @@ -1,80 +0,0 @@ -/* - * Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. Oracle designates this - * particular file as subject to the "Classpath" exception as provided - * by Oracle in the LICENSE file that accompanied this code. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - -package org.openjdk.nashorn.internal.runtime.linker; - -import org.openjdk.nashorn.internal.runtime.ECMAErrors; -import org.openjdk.nashorn.internal.runtime.ECMAException; - -/** - * A result of generating an adapter for a class. A tuple of an outcome and - in case of an error outcome - a list of - * classes that caused the error. - */ -final class AdaptationResult { - /** - * Contains various outcomes for attempting to generate an adapter class. These are stored in AdapterInfo instances. - * We have a successful outcome (adapter class was generated) and four possible error outcomes: superclass is final, - * superclass is not public, superclass has no public or protected constructor, more than one superclass was - * specified. We don't throw exceptions when we try to generate the adapter, but rather just record these error - * conditions as they are still useful as partial outcomes, as Nashorn's linker can still successfully check whether - * the class can be autoconverted from a script function even when it is not possible to generate an adapter for it. - */ - enum Outcome { - SUCCESS, - ERROR_FINAL_CLASS, - ERROR_NON_PUBLIC_CLASS, - ERROR_NO_ACCESSIBLE_CONSTRUCTOR, - ERROR_MULTIPLE_SUPERCLASSES, - ERROR_DUPLICATE_TYPE, - ERROR_TOO_MANY_INTERFACES, - ERROR_NO_COMMON_LOADER, - ERROR_FINAL_FINALIZER, - ERROR_OTHER - } - - static final AdaptationResult SUCCESSFUL_RESULT = new AdaptationResult(Outcome.SUCCESS, ""); - - private final Outcome outcome; - private final RuntimeException cause; - private final String[] messageArgs; - - AdaptationResult(final Outcome outcome, final RuntimeException cause, final String... messageArgs) { - this.outcome = outcome; - this.cause = cause; - this.messageArgs = messageArgs; - } - - AdaptationResult(final Outcome outcome, final String... messageArgs) { - this(outcome, null, messageArgs); - } - - Outcome getOutcome() { - return outcome; - } - - ECMAException typeError() { - return ECMAErrors.typeError(cause, "extend." + outcome, messageArgs); - } -} diff --git a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterBytecodeGenerator.java b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterBytecodeGenerator.java index 07dbbc458..a29515207 100644 --- a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterBytecodeGenerator.java +++ b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterBytecodeGenerator.java @@ -42,7 +42,6 @@ import static org.openjdk.nashorn.internal.codegen.CompilerConstants.interfaceCallNoLookup; import static org.openjdk.nashorn.internal.codegen.CompilerConstants.staticCallNoLookup; import static org.openjdk.nashorn.internal.lookup.Lookup.MH; -import static org.openjdk.nashorn.internal.runtime.linker.AdaptationResult.Outcome.ERROR_NO_ACCESSIBLE_CONSTRUCTOR; import java.lang.annotation.Annotation; import java.lang.invoke.CallSite; @@ -73,7 +72,6 @@ import org.openjdk.nashorn.internal.codegen.CompilerConstants.Call; import org.openjdk.nashorn.internal.runtime.ScriptFunction; import org.openjdk.nashorn.internal.runtime.ScriptObject; -import org.openjdk.nashorn.internal.runtime.linker.AdaptationResult.Outcome; /** * Generates bytecode for a Java adapter class. Used by the {@link JavaAdapterFactory}. @@ -249,10 +247,9 @@ final class JavaAdapterBytecodeGenerator { * @param commonLoader the class loader that can see all of superClass, interfaces, and Nashorn classes. * @param classOverride true to generate the bytecode for the adapter that has class-level overrides, false to * generate the bytecode for the adapter that has instance-level overrides. - * @throws AdaptationException if the adapter can not be generated for some reason. */ JavaAdapterBytecodeGenerator(final Class superClass, final List> interfaces, - final ClassLoader commonLoader, final boolean classOverride) throws AdaptationException { + final ClassLoader commonLoader, final boolean classOverride) { assert superClass != null && !superClass.isInterface(); assert interfaces != null; @@ -383,7 +380,7 @@ private void emitInitCallThis(final InstructionAdapter mv) { } } - private boolean generateConstructors() throws AdaptationException { + private boolean generateConstructors() { boolean gotCtor = false; boolean canBeAutoConverted = false; for (final Constructor ctor: superClass.getDeclaredConstructors()) { @@ -394,7 +391,8 @@ private boolean generateConstructors() throws AdaptationException { } } if(!gotCtor) { - throw new AdaptationException(ERROR_NO_ACCESSIBLE_CONSTRUCTOR, superClass.getCanonicalName()); + throw JavaAdapterFactory.adaptationException( + JavaAdapterFactory.ErrorOutcome.NO_ACCESSIBLE_CONSTRUCTOR, superClass.getCanonicalName()); } return canBeAutoConverted; } @@ -1123,7 +1121,7 @@ private static int getAccessModifiers(final Method method) { * class. * @param type the type defining the methods. */ - private void gatherMethods(final Class type) throws AdaptationException { + private void gatherMethods(final Class type) { if (Modifier.isPublic(type.getModifiers())) { final Method[] typeMethods = type.isInterface() ? type.getMethods() : type.getDeclaredMethods(); @@ -1143,7 +1141,8 @@ private void gatherMethods(final Class type) throws AdaptationException { hasExplicitFinalizer = true; if(Modifier.isFinal(m)) { // Must be able to override an explicit finalizer - throw new AdaptationException(Outcome.ERROR_FINAL_FINALIZER, type.getCanonicalName()); + throw JavaAdapterFactory.adaptationException( + JavaAdapterFactory.ErrorOutcome.FINAL_FINALIZER, type.getCanonicalName()); } } continue; @@ -1174,7 +1173,7 @@ private void gatherMethods(final Class type) throws AdaptationException { } } - private void gatherMethods(final List> classes) throws AdaptationException { + private void gatherMethods(final List> classes) { for(final Class c: classes) { gatherMethods(c); } diff --git a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java index 106034168..12d168b7b 100644 --- a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java +++ b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java @@ -50,10 +50,10 @@ import jdk.dynalink.beans.StaticClass; import jdk.dynalink.linker.support.SimpleLinkRequest; import org.openjdk.nashorn.internal.runtime.Context; +import org.openjdk.nashorn.internal.runtime.ECMAErrors; import org.openjdk.nashorn.internal.runtime.ECMAException; import org.openjdk.nashorn.internal.runtime.ScriptFunction; import org.openjdk.nashorn.internal.runtime.ScriptObject; -import org.openjdk.nashorn.internal.runtime.linker.AdaptationResult.Outcome; /** * A factory class that generates adapter classes. Adapter classes allow @@ -98,6 +98,17 @@ protected Map>, AdapterInfo> computeValue(final Class type) { } }; + private static final ClassValue AUTO_CONVERTIBLE_FROM_FUNCTION = new ClassValue<>() { + @Override + protected Boolean computeValue(final Class type) { + try { + return getAdapterInfo(new Class[] { type }).autoConvertibleFromFunction; + } catch (Exception e) { + return false; + } + } + }; + /** * Returns an adapter class for the specified original types. The adapter * class extends/implements the original class/interfaces. @@ -205,7 +216,7 @@ public static MethodHandle getConstructor(final Class sourceType, final Class * be generated from a ScriptFunction. */ static boolean isAutoConvertibleFromFunction(final Class clazz) { - return getAdapterInfo(new Class[] { clazz }).isAutoConvertibleFromFunction(); + return AUTO_CONVERTIBLE_FROM_FUNCTION.get(clazz); } private static AdapterInfo getAdapterInfo(final Class[] types) { @@ -230,45 +241,49 @@ private static AdapterInfo createAdapterInfo(final List> types, final C final int mod = t.getModifiers(); if(!t.isInterface()) { if (superClass == t) { - throw new AdaptationResult(Outcome.ERROR_DUPLICATE_TYPE, t.getCanonicalName()).typeError(); + throw adaptationException(ErrorOutcome.DUPLICATE_TYPE, t.getCanonicalName()); } else if(superClass != null) { - throw new AdaptationResult(Outcome.ERROR_MULTIPLE_SUPERCLASSES, t.getCanonicalName() + " and " + superClass.getCanonicalName()).typeError(); + throw adaptationException(ErrorOutcome.MULTIPLE_SUPERCLASSES, t.getCanonicalName() + " and " + superClass.getCanonicalName()); } else if (Modifier.isFinal(mod)) { - return new AdapterInfo(AdaptationResult.Outcome.ERROR_FINAL_CLASS, t.getCanonicalName()); + throw adaptationException(ErrorOutcome.FINAL_CLASS, t.getCanonicalName()); } superClass = t; } else { if (interfaces.size() > 65535) { - throw new AdaptationResult(Outcome.ERROR_TOO_MANY_INTERFACES, "65535").typeError(); + throw adaptationException(ErrorOutcome.TOO_MANY_INTERFACES, "65535"); } else if (!interfacesDedup.add(t)) { - throw new AdaptationResult(Outcome.ERROR_DUPLICATE_TYPE, t.getCanonicalName()).typeError(); + throw adaptationException(ErrorOutcome.DUPLICATE_TYPE, t.getCanonicalName()); } interfaces.add(t); } if(!Modifier.isPublic(mod)) { - return new AdapterInfo(AdaptationResult.Outcome.ERROR_NON_PUBLIC_CLASS, t.getCanonicalName()); + throw adaptationException(ErrorOutcome.NON_PUBLIC_CLASS, t.getCanonicalName()); } } final Class effectiveSuperClass = superClass == null ? Object.class : superClass; - return AccessController.doPrivileged((PrivilegedAction) () -> { - try { - try { - return new AdapterInfo(effectiveSuperClass, interfaces, definingClassAndLoader); - } catch (final RuntimeException e) { - throw new AdaptationException(new AdaptationResult(Outcome.ERROR_OTHER, e, types.toString(), e.toString())); - } - } catch (final AdaptationException e) { - final var res = e.getAdaptationResult(); - if (types.size() > 1) { - // Only cache error outcomes for single types - throw res.typeError(); - } else { - return new AdapterInfo(res); - } - } - }, CREATE_ADAPTER_INFO_ACC_CTXT); + return AccessController.doPrivileged((PrivilegedAction) () -> + new AdapterInfo(effectiveSuperClass, interfaces, definingClassAndLoader), + CREATE_ADAPTER_INFO_ACC_CTXT); + } + + static ECMAException adaptationException(ErrorOutcome outcome, String... messageArgs) { + return ECMAErrors.typeError("extend." + outcome, messageArgs); + } + + /** + * Contains various error outcomes for attempting to generate an adapter class. + */ + enum ErrorOutcome { + FINAL_CLASS, + NON_PUBLIC_CLASS, + NO_ACCESSIBLE_CONSTRUCTOR, + MULTIPLE_SUPERCLASSES, + DUPLICATE_TYPE, + TOO_MANY_INTERFACES, + NO_COMMON_LOADER, + FINAL_FINALIZER } private static class AdapterInfo { @@ -280,60 +295,33 @@ private static class AdapterInfo { private final JavaAdapterClassLoader instanceAdapterGenerator; private final Map instanceAdapters = new ConcurrentHashMap<>(); final boolean autoConvertibleFromFunction; - final AdaptationResult adaptationResult; - AdapterInfo(final Class superClass, final List> interfaces, final ClassAndLoader definingLoader) throws AdaptationException { + AdapterInfo(final Class superClass, final List> interfaces, final ClassAndLoader definingLoader) { this.commonLoader = findCommonLoader(definingLoader); final JavaAdapterBytecodeGenerator gen = new JavaAdapterBytecodeGenerator(superClass, interfaces, commonLoader, false); this.autoConvertibleFromFunction = gen.isAutoConvertibleFromFunction(); instanceAdapterGenerator = gen.createAdapterClassLoader(); this.classAdapterGenerator = new JavaAdapterBytecodeGenerator(superClass, interfaces, commonLoader, true).createAdapterClassLoader(); - this.adaptationResult = AdaptationResult.SUCCESSFUL_RESULT; - } - - AdapterInfo(final AdaptationResult.Outcome outcome, final String classList) { - this(new AdaptationResult(outcome, classList)); - } - - AdapterInfo(final AdaptationResult adaptationResult) { - this.commonLoader = null; - this.classAdapterGenerator = null; - this.instanceAdapterGenerator = null; - this.autoConvertibleFromFunction = false; - this.adaptationResult = adaptationResult; } StaticClass getAdapterClass(final ScriptObject classOverrides, final ProtectionDomain protectionDomain) { - if(adaptationResult.getOutcome() != AdaptationResult.Outcome.SUCCESS) { - throw adaptationResult.typeError(); - } return classOverrides == null ? getInstanceAdapterClass(protectionDomain) : getClassAdapterClass(classOverrides, protectionDomain); } - boolean isAutoConvertibleFromFunction() { - if(adaptationResult.getOutcome() == AdaptationResult.Outcome.ERROR_OTHER) { - throw adaptationResult.typeError(); - } - return autoConvertibleFromFunction; - } - private StaticClass getInstanceAdapterClass(final ProtectionDomain protectionDomain) { CodeSource codeSource = protectionDomain.getCodeSource(); if(codeSource == null) { codeSource = MINIMAL_PERMISSION_DOMAIN.getCodeSource(); } - StaticClass instanceAdapterClass = instanceAdapters.get(codeSource); - if(instanceAdapterClass != null) { - return instanceAdapterClass; - } - // Any "unknown source" code source will default to no permission domain. - final ProtectionDomain effectiveDomain = codeSource.equals(MINIMAL_PERMISSION_DOMAIN.getCodeSource()) ? - MINIMAL_PERMISSION_DOMAIN : protectionDomain; + return instanceAdapters.computeIfAbsent(codeSource, cs -> { + // Any "unknown source" code source will default to no permission domain. + final ProtectionDomain effectiveDomain = + cs.equals(MINIMAL_PERMISSION_DOMAIN.getCodeSource()) + ? MINIMAL_PERMISSION_DOMAIN : protectionDomain; - instanceAdapterClass = instanceAdapterGenerator.generateClass(commonLoader, effectiveDomain); - final StaticClass existing = instanceAdapters.putIfAbsent(codeSource, instanceAdapterClass); - return existing == null ? instanceAdapterClass : existing; + return instanceAdapterGenerator.generateClass(commonLoader, effectiveDomain); + }); } private StaticClass getClassAdapterClass(final ScriptObject classOverrides, final ProtectionDomain protectionDomain) { @@ -353,10 +341,8 @@ private StaticClass getClassAdapterClass(final ScriptObject classOverrides, fina * be used to add the generated adapter to its ADAPTER_INFO_MAPS. * * @return the class loader that sees both the specified class and Nashorn classes. - * - * @throws IllegalStateException if no such class loader is found. */ - private static ClassLoader findCommonLoader(final ClassAndLoader classAndLoader) throws AdaptationException { + private static ClassLoader findCommonLoader(final ClassAndLoader classAndLoader) { if(classAndLoader.canSee(SCRIPT_OBJECT_LOADER)) { return classAndLoader.getLoader(); } @@ -364,7 +350,7 @@ private static ClassLoader findCommonLoader(final ClassAndLoader classAndLoader) return SCRIPT_OBJECT_LOADER.getLoader(); } - throw new AdaptationException(AdaptationResult.Outcome.ERROR_NO_COMMON_LOADER, classAndLoader.getRepresentativeClass().getCanonicalName()); + throw adaptationException(ErrorOutcome.NO_COMMON_LOADER, classAndLoader.getRepresentativeClass().getCanonicalName()); } } diff --git a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/resources/Messages.properties b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/resources/Messages.properties index c355c4833..0f7f22b10 100644 --- a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/resources/Messages.properties +++ b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/resources/Messages.properties @@ -170,15 +170,14 @@ type.error.extend.expects.at.least.one.argument=Java.extend needs at least one a type.error.extend.expects.at.least.one.type.argument=Java.extend needs at least one type argument. type.error.extend.expects.java.types=Java.extend needs Java types as its arguments. type.error.extend.ambiguous.defining.class=There is no class loader that can see all of {0} at once. -type.error.extend.ERROR_FINAL_CLASS=Can not extend final class {0}. -type.error.extend.ERROR_NON_PUBLIC_CLASS=Can not extend/implement non-public class/interface {0}. -type.error.extend.ERROR_NO_ACCESSIBLE_CONSTRUCTOR=Can not extend class {0} as it has no public or protected constructors. -type.error.extend.ERROR_MULTIPLE_SUPERCLASSES=Can not extend multiple classes {0}. At most one of the specified types can be a class, the rest must all be interfaces. -type.error.extend.ERROR_DUPLICATE_TYPE=Type {0} specified multiple times. -type.error.extend.ERROR_TOO_MANY_INTERFACES=Too many interfaces specified. At most {0} are supported by the JVM. -type.error.extend.ERROR_NO_COMMON_LOADER=Can not find a common class loader for ScriptObject and {0}. -type.error.extend.ERROR_FINAL_FINALIZER=Can not extend class because {0} has a final finalize method. -type.error.extend.ERROR_OTHER=Can not extend/implement {0} because of {1} +type.error.extend.FINAL_CLASS=Can not extend final class {0}. +type.error.extend.NON_PUBLIC_CLASS=Can not extend/implement non-public class/interface {0}. +type.error.extend.NO_ACCESSIBLE_CONSTRUCTOR=Can not extend class {0} as it has no public or protected constructors. +type.error.extend.MULTIPLE_SUPERCLASSES=Can not extend multiple classes {0}. At most one of the specified types can be a class, the rest must all be interfaces. +type.error.extend.DUPLICATE_TYPE=Type {0} specified multiple times. +type.error.extend.TOO_MANY_INTERFACES=Too many interfaces specified. At most {0} are supported by the JVM. +type.error.extend.NO_COMMON_LOADER=Can not find a common class loader for ScriptObject and {0}. +type.error.extend.FINAL_FINALIZER=Can not extend class because {0} has a final finalize method. type.error.no.constructor.matches.args=Can not construct {0} with the passed arguments; they do not match any of its constructor signatures. type.error.no.method.matches.args=Can not invoke method {0} with the passed arguments; they do not match any of its method signatures. type.error.no.constructor.matches.args=Can not create new object with constructor {0} with the passed arguments; they do not match any of its method signatures. From 2438106e72693b351d554a227c4f5a57fe89019a Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Sun, 21 Mar 2021 18:38:24 +0100 Subject: [PATCH 3/3] Lazily create AdapterInfo.instanceAdapters map. --- .../runtime/linker/JavaAdapterFactory.java | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java index 12d168b7b..f63b643c1 100644 --- a/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java +++ b/src/org.openjdk.nashorn/share/classes/org/openjdk/nashorn/internal/runtime/linker/JavaAdapterFactory.java @@ -31,6 +31,7 @@ import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodHandles.Lookup; import java.lang.invoke.MethodType; +import java.lang.invoke.VarHandle; import java.lang.reflect.Modifier; import java.security.AccessControlContext; import java.security.AccessController; @@ -288,12 +289,20 @@ enum ErrorOutcome { private static class AdapterInfo { private static final ClassAndLoader SCRIPT_OBJECT_LOADER = new ClassAndLoader(ScriptFunction.class, true); + private static final VarHandle INSTANCE_ADAPTERS; + static { + try { + INSTANCE_ADAPTERS = MethodHandles.lookup().findVarHandle(AdapterInfo.class, "instanceAdapters", Map.class); + } catch (ReflectiveOperationException e) { + throw new RuntimeException(e); + } + } private final ClassLoader commonLoader; // TODO: soft reference the JavaAdapterClassLoader objects. They can be recreated when needed. private final JavaAdapterClassLoader classAdapterGenerator; private final JavaAdapterClassLoader instanceAdapterGenerator; - private final Map instanceAdapters = new ConcurrentHashMap<>(); + private Map instanceAdapters; final boolean autoConvertibleFromFunction; AdapterInfo(final Class superClass, final List> interfaces, final ClassAndLoader definingLoader) { @@ -314,7 +323,14 @@ private StaticClass getInstanceAdapterClass(final ProtectionDomain protectionDom if(codeSource == null) { codeSource = MINIMAL_PERMISSION_DOMAIN.getCodeSource(); } - return instanceAdapters.computeIfAbsent(codeSource, cs -> { + var ia = instanceAdapters; + if (ia == null) { + var nia = new ConcurrentHashMap(); + @SuppressWarnings("unchecked") + var xia = (Map)INSTANCE_ADAPTERS.compareAndExchange(this, null, nia); + ia = xia == null ? nia : xia; + } + return ia.computeIfAbsent(codeSource, cs -> { // Any "unknown source" code source will default to no permission domain. final ProtectionDomain effectiveDomain = cs.equals(MINIMAL_PERMISSION_DOMAIN.getCodeSource())