8202113: Reflection API is causing caller classes to leak
Reviewed-by: alanb, plevart
--- a/src/java.base/share/classes/java/lang/reflect/AccessibleObject.java Thu May 10 13:34:44 2018 -0700
+++ b/src/java.base/share/classes/java/lang/reflect/AccessibleObject.java Fri May 11 14:21:46 2018 -0700
@@ -564,7 +564,6 @@
throw new AssertionError("All subclasses should override this method");
}
-
// Shared access checking logic.
// For non-public members or members in package-private classes,
@@ -674,4 +673,13 @@
}
return printStackWhenAccessFails;
}
+
+ /**
+ * Returns the root AccessibleObject; or null if this object is the root.
+ *
+ * All subclasses override this method.
+ */
+ AccessibleObject getRoot() {
+ throw new InternalError();
+ }
}
--- a/src/java.base/share/classes/java/lang/reflect/Constructor.java Thu May 10 13:34:44 2018 -0700
+++ b/src/java.base/share/classes/java/lang/reflect/Constructor.java Fri May 11 14:21:46 2018 -0700
@@ -103,11 +103,8 @@
// occur in annotation code.
private Constructor<T> root;
- /**
- * Used by Excecutable for annotation sharing.
- */
@Override
- Executable getRoot() {
+ Constructor<T> getRoot() {
return root;
}
--- a/src/java.base/share/classes/java/lang/reflect/Executable.java Thu May 10 13:34:44 2018 -0700
+++ b/src/java.base/share/classes/java/lang/reflect/Executable.java Fri May 11 14:21:46 2018 -0700
@@ -56,11 +56,6 @@
abstract byte[] getAnnotationBytes();
/**
- * Accessor method to allow code sharing
- */
- abstract Executable getRoot();
-
- /**
* Does the Executable have generic information.
*/
abstract boolean hasGenericInformation();
@@ -602,7 +597,7 @@
if ((declAnnos = declaredAnnotations) == null) {
synchronized (this) {
if ((declAnnos = declaredAnnotations) == null) {
- Executable root = getRoot();
+ Executable root = (Executable)getRoot();
if (root != null) {
declAnnos = root.declaredAnnotations();
} else {
--- a/src/java.base/share/classes/java/lang/reflect/Field.java Thu May 10 13:34:44 2018 -0700
+++ b/src/java.base/share/classes/java/lang/reflect/Field.java Fri May 11 14:21:46 2018 -0700
@@ -1128,6 +1128,11 @@
}
}
+ @Override
+ Field getRoot() {
+ return root;
+ }
+
/**
* @throws NullPointerException {@inheritDoc}
* @since 1.5
--- a/src/java.base/share/classes/java/lang/reflect/Method.java Thu May 10 13:34:44 2018 -0700
+++ b/src/java.base/share/classes/java/lang/reflect/Method.java Fri May 11 14:21:46 2018 -0700
@@ -198,11 +198,8 @@
checkCanSetAccessible(caller, clazz);
}
- /**
- * Used by Excecutable for annotation sharing.
- */
@Override
- Executable getRoot() {
+ Method getRoot() {
return root;
}
--- a/src/java.base/share/classes/java/lang/reflect/ReflectAccess.java Thu May 10 13:34:44 2018 -0700
+++ b/src/java.base/share/classes/java/lang/reflect/ReflectAccess.java Fri May 11 14:21:46 2018 -0700
@@ -154,4 +154,9 @@
public <T> Constructor<T> copyConstructor(Constructor<T> arg) {
return arg.copy();
}
+
+ @SuppressWarnings("unchecked")
+ public <T extends AccessibleObject> T getRoot(T obj) {
+ return (T) obj.getRoot();
+ }
}
--- a/src/java.base/share/classes/jdk/internal/reflect/LangReflectAccess.java Thu May 10 13:34:44 2018 -0700
+++ b/src/java.base/share/classes/jdk/internal/reflect/LangReflectAccess.java Fri May 11 14:21:46 2018 -0700
@@ -115,4 +115,7 @@
/** Makes a "child" copy of a Constructor */
public <T> Constructor<T> copyConstructor(Constructor<T> arg);
+
+ /** Gets the root of the given AccessibleObject object; null if arg is the root */
+ public <T extends AccessibleObject> T getRoot(T obj);
}
--- a/src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java Thu May 10 13:34:44 2018 -0700
+++ b/src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java Fri May 11 14:21:46 2018 -0700
@@ -39,7 +39,6 @@
import java.lang.reflect.Method;
import java.lang.reflect.Constructor;
import java.lang.reflect.Modifier;
-import java.security.Permission;
import java.security.PrivilegedAction;
import java.util.Objects;
import java.util.Properties;
@@ -172,6 +171,15 @@
*/
public FieldAccessor newFieldAccessor(Field field, boolean override) {
checkInitted();
+
+ Field root = langReflectAccess.getRoot(field);
+ if (root != null) {
+ // FieldAccessor will use the root unless the modifiers have
+ // been overrridden
+ if (root.getModifiers() == field.getModifiers() || !override) {
+ field = root;
+ }
+ }
return UnsafeFieldAccessorFactory.newFieldAccessor(field, override);
}
@@ -185,6 +193,12 @@
}
}
+ // use the root Method that will not cache caller class
+ Method root = langReflectAccess.getRoot(method);
+ if (root != null) {
+ method = root;
+ }
+
if (noInflation && !ReflectUtil.isVMAnonymousClass(method.getDeclaringClass())) {
return new MethodAccessorGenerator().
generateMethod(method.getDeclaringClass(),
@@ -214,6 +228,13 @@
return new InstantiationExceptionConstructorAccessorImpl
("Can not instantiate java.lang.Class");
}
+
+ // use the root Constructor that will not cache caller class
+ Constructor<?> root = langReflectAccess.getRoot(c);
+ if (root != null) {
+ c = root;
+ }
+
// Bootstrapping issue: since we use Class.newInstance() in
// the ConstructorAccessor generation process, we have to
// break the cycle here.
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/lang/reflect/callerCache/AccessTest.java Fri May 11 14:21:46 2018 -0700
@@ -0,0 +1,171 @@
+/*
+ * Copyright (c) 2018, 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.
+ */
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.util.concurrent.Callable;
+
+/**
+ * Each nested class tests a member of a specific access.
+ *
+ * Caller class is cached when setAccessible is not used to suppress the access
+ * check and only access allowed. If not accessible, caller class is not cached.
+ */
+public class AccessTest {
+ public static class PublicConstructor implements Callable<Object> {
+ public Object call() throws Exception {
+ Constructor c = Members.class.getConstructor();
+ return c.newInstance();
+ }
+ }
+
+ public static class PublicMethod extends Members implements Callable<Void> {
+ public Void call() throws Exception {
+ Method m = Members.class.getDeclaredMethod("publicMethod");
+ m.invoke(new PublicMethod());
+ return null;
+ }
+ }
+
+ public static class ProtectedMethod extends Members implements Callable<Void> {
+ public Void call() throws Exception {
+ Method m = Members.class.getDeclaredMethod("protectedMethod");
+ m.invoke(new ProtectedMethod());
+ return null;
+ }
+ }
+
+ /*
+ * private field is not accessible. So caller class is not cached.
+ */
+ public static class PrivateMethod extends Members implements Callable<Void> {
+ public Void call() throws Exception {
+ Method m = Members.class.getDeclaredMethod("privateMethod");
+ try {
+ m.invoke(new ProtectedMethod());
+ } catch (IllegalAccessException e) {
+ }
+ return null;
+ }
+ }
+
+ public static class PublicField extends Members implements Callable<Void> {
+ public Void call() throws Exception {
+ Field f = Members.class.getDeclaredField("publicField");
+ f.get(new PublicField());
+ return null;
+ }
+ }
+
+ public static class ProtectedField extends Members implements Callable<Void> {
+ public Void call() throws Exception {
+ Field f = Members.class.getDeclaredField("protectedField");
+ f.get(new ProtectedField());
+ return null;
+ }
+ }
+
+ /*
+ * private field is not accessible. So caller class is not cached.
+ */
+ public static class PrivateField implements Callable<Void> {
+ public Void call() throws Exception {
+ Field f = Members.class.getDeclaredField("privateField");
+ try {
+ f.get(new Members());
+ } catch (IllegalAccessException e) {
+ }
+ return null;
+ }
+ }
+
+ /*
+ * Validate final field
+ */
+ public static class FinalField implements Callable<Void> {
+ final Field f;
+ final boolean isStatic;
+ public FinalField(String name) throws Exception {
+ this.f = Members.class.getDeclaredField(name);
+ this.isStatic = Modifier.isStatic(f.getModifiers());
+ if (!Modifier.isFinal(f.getModifiers())) {
+ throw new RuntimeException("not a final field");
+ }
+ makeFinalNonFinal(f);
+ }
+ public Void call() throws Exception {
+ Members obj = isStatic ? null : new Members();
+ try {
+ f.set(obj, 20);
+ checkValue(obj, 20);
+ } catch (IllegalAccessException e) {
+ throw e;
+ }
+ return null;
+ }
+
+ void checkValue(Object obj, int expected) throws Exception {
+ int value = (int) f.get(obj);
+ if (value != expected) {
+ throw new RuntimeException("unexpectd value: " + value);
+ }
+ }
+ }
+
+ public static class PublicFinalField extends FinalField {
+ public PublicFinalField() throws Exception {
+ super("publicFinalField");
+ }
+ }
+
+ public static class PrivateFinalField extends FinalField {
+ public PrivateFinalField() throws Exception {
+ super("privateFinalField");
+ }
+ }
+
+ public static class PublicStaticFinalField extends FinalField {
+ public PublicStaticFinalField() throws Exception {
+ super("publicStaticFinalField");
+ }
+ }
+
+ public static class PrivateStaticFinalField extends FinalField {
+ public PrivateStaticFinalField() throws Exception {
+ super("privateStaticFinalField");
+ }
+ }
+
+ private static void makeFinalNonFinal(Field f) throws ReflectiveOperationException {
+ Field modifiers = Field.class.getDeclaredField("modifiers");
+ modifiers.setAccessible(true);
+ modifiers.set(f, modifiers.getInt(f) & ~Modifier.FINAL);
+ f.setAccessible(true);
+
+ if (Modifier.isFinal(f.getModifiers())) {
+ throw new RuntimeException("should be a non-final field");
+ }
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/lang/reflect/callerCache/Members.java Fri May 11 14:21:46 2018 -0700
@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2018, 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.
+ */
+
+public class Members {
+ public Members() {}
+ protected Members(boolean b) {}
+ private Members(int i) {}
+
+ public void publicMethod() {}
+ protected void protectedMethod() {}
+ private void privateMethod() {}
+
+ public Object publicField = new Object();
+ protected Object protectedField = new Object();
+ private Object privateField = new Object();
+
+ public final int publicFinalField = 10;
+ private final int privateFinalField = 10;
+ public static final int publicStaticFinalField = 10;
+ private static final int privateStaticFinalField = 10;
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/lang/reflect/callerCache/ReflectionCallerCacheTest.java Fri May 11 14:21:46 2018 -0700
@@ -0,0 +1,175 @@
+/*
+ * Copyright (c) 2018, 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.
+ */
+
+/*
+ * @test
+ * @bug 8202113
+ * @summary Test the caller class loader is not kept strongly reachable
+ * by reflection API
+ * @library /test/lib/
+ * @build ReflectionCallerCacheTest Members jdk.test.lib.compiler.CompilerUtils
+ * @run testng/othervm ReflectionCallerCacheTest
+ */
+
+import java.io.IOException;
+import java.lang.ref.Cleaner;
+import java.lang.ref.WeakReference;
+import java.lang.reflect.*;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.function.BooleanSupplier;
+
+import jdk.test.lib.compiler.CompilerUtils;
+import org.testng.annotations.BeforeTest;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+public class ReflectionCallerCacheTest {
+ private static final Path CLASSES = Paths.get("classes");
+ private static final ReflectionCallerCacheTest TEST = new ReflectionCallerCacheTest();
+
+ @BeforeTest
+ public void setup() throws IOException {
+ String src = System.getProperty("test.src", ".");
+ String classpath = System.getProperty("test.classes", ".");
+ boolean rc = CompilerUtils.compile(Paths.get(src, "AccessTest.java"), CLASSES, "-cp", classpath);
+ if (!rc) {
+ throw new RuntimeException("fail compilation");
+ }
+ }
+ @DataProvider(name = "memberAccess")
+ public Object[][] memberAccess() {
+ return new Object[][] {
+ { "AccessTest$PublicConstructor" },
+ { "AccessTest$PublicMethod" },
+ { "AccessTest$PublicField" },
+ { "AccessTest$ProtectedMethod" },
+ { "AccessTest$ProtectedField" },
+ { "AccessTest$PrivateMethod" },
+ { "AccessTest$PrivateField"},
+ { "AccessTest$PublicFinalField"},
+ { "AccessTest$PrivateFinalField"},
+ { "AccessTest$PublicStaticFinalField"},
+ { "AccessTest$PrivateStaticFinalField"}
+ };
+ }
+
+ // Keep the root of the reflective objects strongly reachable
+ private final Constructor<?> publicConstructor;
+ private final Method publicMethod;
+ private final Method protectedMethod;
+ private final Method privateMethod;
+ private final Field publicField;
+ private final Field protectedField;
+ private final Field privateField;
+
+ ReflectionCallerCacheTest() {
+ try {
+ this.publicConstructor = Members.class.getConstructor();
+ this.publicMethod = Members.class.getDeclaredMethod("publicMethod");
+ this.publicField = Members.class.getDeclaredField("publicField");
+ this.protectedMethod = Members.class.getDeclaredMethod("protectedMethod");
+ this.protectedField = Members.class.getDeclaredField("protectedField");
+ this.privateMethod = Members.class.getDeclaredMethod("privateMethod");
+ this.privateField = Members.class.getDeclaredField("privateField");
+ } catch (ReflectiveOperationException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ @Test(dataProvider = "memberAccess")
+ private void load(String classname) throws Exception {
+ WeakReference<?> weakLoader = loadAndRunClass(classname);
+
+ // Force garbage collection to trigger unloading of class loader
+ new ForceGC().await(() -> weakLoader.get() == null);
+
+ if (weakLoader.get() != null) {
+ throw new RuntimeException("Class " + classname + " not unloaded!");
+ }
+ }
+
+ private WeakReference<?> loadAndRunClass(String classname) throws Exception {
+ try (TestLoader loader = new TestLoader()) {
+ // Load member access class with custom class loader
+ Class<?> c = Class.forName(classname, true, loader);
+ // access the reflective member
+ Callable callable = (Callable) c.newInstance();
+ callable.call();
+ return new WeakReference<>(loader);
+ }
+ }
+
+ static class TestLoader extends URLClassLoader {
+ static URL[] toURLs() {
+ try {
+ return new URL[] { CLASSES.toUri().toURL() };
+ } catch (MalformedURLException e) {
+ throw new Error(e);
+ }
+ }
+
+ TestLoader() {
+ super("testloader", toURLs(), ClassLoader.getSystemClassLoader());
+ }
+ }
+
+ /**
+ * Utility class to invoke System.gc()
+ */
+ static class ForceGC {
+ private final CountDownLatch cleanerInvoked = new CountDownLatch(1);
+ private final Cleaner cleaner = Cleaner.create();
+
+ ForceGC() {
+ cleaner.register(new Object(), () -> cleanerInvoked.countDown());
+ }
+
+ void doit() {
+ try {
+ for (int i = 0; i < 10; i++) {
+ System.gc();
+ if (cleanerInvoked.await(1L, TimeUnit.SECONDS)) {
+ return;
+ }
+ }
+ } catch (InterruptedException unexpected) {
+ throw new AssertionError("unexpected InterruptedException");
+ }
+ }
+
+ void await(BooleanSupplier s) {
+ for (int i = 0; i < 10; i++) {
+ if (s.getAsBoolean()) return;
+ doit();
+ }
+ throw new AssertionError("failed to satisfy condition");
+ }
+ }
+}