From c5b133e6b13ea468763730e45f872b2370c2a206 Mon Sep 17 00:00:00 2001 From: Stephan Ewen Date: Sun, 28 Jul 2019 17:16:17 +0200 Subject: [PATCH] [FLINK-13451][tests] Remove use of Unsafe.defineClass() from CommonTestUtils The method Unsafe.defineClass() is removed in Java 11. To support Java 11, we rework the method "CommonTestUtils.createClassNotInClassPath()" to use a different mechanism. This commit now writes the class byte code out to a temporary file and create a new URLClassLoader that loads the class from that file. That solution is not a complete drop-in replacement, because it cannot add the class to an existing class loader, but can only create a new pair of (classloader & new-class-in-that-classloader). Because of that, the commit also adjusts the existing tests to work with that new mechanism. This closes #9251 --- .../kryo/KryoSerializerClassLoadingTest.java | 23 +-- .../kryo/KryoSerializerSnapshotTest.java | 20 +- .../CheckpointSettingsSerializableTest.java | 7 +- .../runtime/executiongraph/ErrorInfoTest.java | 6 +- .../runtime/state/JavaSerializerTest.java | 22 +-- .../flink/core/testutils/CommonTestUtils.java | 177 +++++++++++++----- .../core/testutils/CommonTestUtilsTest.java | 53 ++++++ 7 files changed, 203 insertions(+), 105 deletions(-) create mode 100644 flink-test-utils-parent/flink-test-utils-junit/src/test/java/org/apache/flink/core/testutils/CommonTestUtilsTest.java diff --git a/flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/kryo/KryoSerializerClassLoadingTest.java b/flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/kryo/KryoSerializerClassLoadingTest.java index 9823e114f7bfa..0d5e9080a217c 100644 --- a/flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/kryo/KryoSerializerClassLoadingTest.java +++ b/flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/kryo/KryoSerializerClassLoadingTest.java @@ -23,14 +23,11 @@ import org.apache.flink.api.common.typeutils.TypeSerializer; import org.apache.flink.api.java.tuple.Tuple1; import org.apache.flink.core.testutils.CommonTestUtils; + import org.junit.After; import org.junit.Before; import org.junit.Test; -import java.io.Serializable; -import java.net.URL; -import java.net.URLClassLoader; - import static org.junit.Assert.fail; /** @@ -39,13 +36,9 @@ */ public class KryoSerializerClassLoadingTest extends SerializerTestBase { - /** Class loader for the object that is not in the test class path */ - private static final ClassLoader CLASS_LOADER = - new URLClassLoader(new URL[0], KryoSerializerClassLoadingTest.class.getClassLoader()); - - /** An object that is not in the test class path */ - private static final Serializable OBJECT_OUT_OF_CLASSPATH = - CommonTestUtils.createObjectForClassNotInClassPath(CLASS_LOADER); + /** Class loader and object that is not in the test class path. */ + private static final CommonTestUtils.ObjectAndClassLoader OUTSIDE_CLASS_LOADING = + CommonTestUtils.createObjectFromNewClassLoader(); // ------------------------------------------------------------------------ @@ -54,7 +47,7 @@ public class KryoSerializerClassLoadingTest extends SerializerTestBase { @Before public void setupClassLoader() { originalClassLoader = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(CLASS_LOADER); + Thread.currentThread().setContextClassLoader(OUTSIDE_CLASS_LOADING.getClassLoader()); } @After @@ -67,7 +60,7 @@ public void restoreOriginalClassLoader() { @Test public void guardTestAssumptions() { try { - Class.forName(OBJECT_OUT_OF_CLASSPATH.getClass().getName()); + Class.forName(OUTSIDE_CLASS_LOADING.getObject().getClass().getName()); fail("This test's assumptions are broken"); } catch (ClassNotFoundException ignored) { @@ -98,11 +91,11 @@ protected Object[] getTestData() { new Integer(7), // an object whose class is not on the classpath - OBJECT_OUT_OF_CLASSPATH, + OUTSIDE_CLASS_LOADING.getObject(), // an object whose class IS on the classpath with a nested object whose class // is NOT on the classpath - new Tuple1<>(OBJECT_OUT_OF_CLASSPATH) + new Tuple1<>(OUTSIDE_CLASS_LOADING.getObject()) }; } diff --git a/flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/kryo/KryoSerializerSnapshotTest.java b/flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/kryo/KryoSerializerSnapshotTest.java index fbfa76540ba72..5435ca529fd4e 100644 --- a/flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/kryo/KryoSerializerSnapshotTest.java +++ b/flink-core/src/test/java/org/apache/flink/api/java/typeutils/runtime/kryo/KryoSerializerSnapshotTest.java @@ -36,8 +36,6 @@ import org.junit.Test; import java.io.IOException; -import java.net.URL; -import java.net.URLClassLoader; import static org.apache.flink.api.common.typeutils.TypeSerializerMatchers.isCompatibleAsIs; import static org.apache.flink.api.common.typeutils.TypeSerializerMatchers.isCompatibleWithReconfiguredSerializer; @@ -129,12 +127,13 @@ private static TypeSerializerSnapshot kryoSnapshotWithMissingClass() thr private static byte[] unLoadableSnapshotBytes() throws IOException { final ClassLoader originalClassLoader = Thread.currentThread().getContextClassLoader(); - ClassLoader tempClassLoader = - new URLClassLoader(new URL[0], KryoSerializerSnapshotTest.class.getClassLoader()); + final CommonTestUtils.ObjectAndClassLoader outsideClassLoading = CommonTestUtils.createObjectFromNewClassLoader(); + try { - Thread.currentThread().setContextClassLoader(tempClassLoader); + Thread.currentThread().setContextClassLoader(outsideClassLoading.getClassLoader()); - ExecutionConfig conf = registerClassThatIsNotInClassPath(tempClassLoader); + ExecutionConfig conf = new ExecutionConfig(); + conf.registerKryoType(outsideClassLoading.getObject().getClass()); KryoSerializer previousSerializer = new KryoSerializer<>(Animal.class, conf); TypeSerializerSnapshot previousSnapshot = previousSerializer.snapshotConfiguration(); @@ -148,15 +147,6 @@ private static byte[] unLoadableSnapshotBytes() throws IOException { } } - private static ExecutionConfig registerClassThatIsNotInClassPath(ClassLoader tempClassLoader) { - Object objectForClassNotInClassPath = - CommonTestUtils.createObjectForClassNotInClassPath(tempClassLoader); - - ExecutionConfig conf = new ExecutionConfig(); - conf.registerKryoType(objectForClassNotInClassPath.getClass()); - return conf; - } - private static TypeSerializerSchemaCompatibility resolveKryoCompatibility(ExecutionConfig previous, ExecutionConfig current) { KryoSerializer previousSerializer = new KryoSerializer<>(Animal.class, previous); TypeSerializerSnapshot previousSnapshot = previousSerializer.snapshotConfiguration(); diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/checkpoint/CheckpointSettingsSerializableTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/checkpoint/CheckpointSettingsSerializableTest.java index e4994b231c093..9a422b36c565e 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/checkpoint/CheckpointSettingsSerializableTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/checkpoint/CheckpointSettingsSerializableTest.java @@ -57,8 +57,6 @@ import javax.annotation.Nonnull; import java.io.IOException; import java.io.Serializable; -import java.net.URL; -import java.net.URLClassLoader; import java.util.Collection; import java.util.Collections; @@ -75,8 +73,9 @@ public class CheckpointSettingsSerializableTest extends TestLogger { @Test public void testDeserializationOfUserCodeWithUserClassLoader() throws Exception { - final ClassLoader classLoader = new URLClassLoader(new URL[0], getClass().getClassLoader()); - final Serializable outOfClassPath = CommonTestUtils.createObjectForClassNotInClassPath(classLoader); + final CommonTestUtils.ObjectAndClassLoader outsideClassLoading = CommonTestUtils.createObjectFromNewClassLoader(); + final ClassLoader classLoader = outsideClassLoading.getClassLoader(); + final Serializable outOfClassPath = outsideClassLoading.getObject(); final MasterTriggerRestoreHook.Factory[] hooks = { new TestFactory(outOfClassPath) }; diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/executiongraph/ErrorInfoTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/executiongraph/ErrorInfoTest.java index 4841365e6b934..63ba4febf7416 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/executiongraph/ErrorInfoTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/executiongraph/ErrorInfoTest.java @@ -23,8 +23,6 @@ import org.junit.Test; import java.io.Serializable; -import java.net.URL; -import java.net.URLClassLoader; import static org.junit.Assert.assertEquals; @@ -50,10 +48,8 @@ private static final class ExceptionWithCustomClassLoader extends Exception { private static final long serialVersionUID = 42L; - private static final ClassLoader CUSTOM_LOADER = new URLClassLoader(new URL[0]); - @SuppressWarnings("unused") - private final Serializable outOfClassLoader = CommonTestUtils.createObjectForClassNotInClassPath(CUSTOM_LOADER); + private final Serializable outOfClassLoader = CommonTestUtils.createObjectFromNewClassLoader().getObject(); public ExceptionWithCustomClassLoader() { super("tada"); diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/state/JavaSerializerTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/state/JavaSerializerTest.java index de6fbce15b8a8..6c0c50c5349f7 100644 --- a/flink-runtime/src/test/java/org/apache/flink/runtime/state/JavaSerializerTest.java +++ b/flink-runtime/src/test/java/org/apache/flink/runtime/state/JavaSerializerTest.java @@ -29,8 +29,6 @@ import java.io.File; import java.io.Serializable; -import java.net.URL; -import java.net.URLClassLoader; import static org.junit.Assert.*; @@ -39,13 +37,9 @@ */ public class JavaSerializerTest extends SerializerTestBase { - /** Class loader for the object that is not in the test class path */ - private static final ClassLoader CLASS_LOADER = - new URLClassLoader(new URL[0], JavaSerializerTest.class.getClassLoader()); - - /** An object that is not in the test class path */ - private static final Serializable OBJECT_OUT_OF_CLASSPATH = - CommonTestUtils.createObjectForClassNotInClassPath(CLASS_LOADER); + /** Class loader and object that is not in the test class path. */ + private static final CommonTestUtils.ObjectAndClassLoader OUTSIDE_CLASS_LOADING = + CommonTestUtils.createObjectFromNewClassLoader(); // ------------------------------------------------------------------------ @@ -54,7 +48,7 @@ public class JavaSerializerTest extends SerializerTestBase { @Before public void setupClassLoader() { originalClassLoader = Thread.currentThread().getContextClassLoader(); - Thread.currentThread().setContextClassLoader(CLASS_LOADER); + Thread.currentThread().setContextClassLoader(OUTSIDE_CLASS_LOADING.getClassLoader()); } @After @@ -68,7 +62,7 @@ public void restoreOriginalClassLoader() { public void guardTest() { // make sure that this test's assumptions hold try { - Class.forName(OBJECT_OUT_OF_CLASSPATH.getClass().getName()); + Class.forName(OUTSIDE_CLASS_LOADING.getObject().getClass().getName()); fail("Test ineffective: The test class that should not be on the classpath is actually on the classpath."); } catch (ClassNotFoundException e) { // expected @@ -79,7 +73,7 @@ public void guardTest() { @Override protected TypeSerializer createSerializer() { - Thread.currentThread().setContextClassLoader(CLASS_LOADER); + Thread.currentThread().setContextClassLoader(OUTSIDE_CLASS_LOADING.getClassLoader()); return new JavaSerializer<>(); } @@ -100,10 +94,10 @@ protected Serializable[] getTestData() { new File("/some/path/that/I/made/up"), // an object that is not in the classpath - OBJECT_OUT_OF_CLASSPATH, + OUTSIDE_CLASS_LOADING.getObject(), // an object that is in the classpath with a nested object not in the classpath - new Tuple1<>(OBJECT_OUT_OF_CLASSPATH) + new Tuple1<>(OUTSIDE_CLASS_LOADING.getObject()) }; } diff --git a/flink-test-utils-parent/flink-test-utils-junit/src/main/java/org/apache/flink/core/testutils/CommonTestUtils.java b/flink-test-utils-parent/flink-test-utils-junit/src/main/java/org/apache/flink/core/testutils/CommonTestUtils.java index 28859f53a237e..5bf1d55f6fb5a 100644 --- a/flink-test-utils-parent/flink-test-utils-junit/src/main/java/org/apache/flink/core/testutils/CommonTestUtils.java +++ b/flink-test-utils-parent/flink-test-utils-junit/src/main/java/org/apache/flink/core/testutils/CommonTestUtils.java @@ -18,6 +18,8 @@ package org.apache.flink.core.testutils; +import javax.annotation.Nullable; + import java.io.BufferedWriter; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -26,13 +28,19 @@ import java.io.IOException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; +import java.io.OutputStream; import java.io.Serializable; import java.lang.reflect.Field; -import java.security.CodeSource; -import java.security.Permissions; -import java.security.ProtectionDomain; -import java.security.cert.Certificate; +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.BasicFileAttributes; import java.util.Map; +import java.util.UUID; /** * This class contains reusable utility methods for unit tests. @@ -150,32 +158,92 @@ public static void setEnv(Map newenv, boolean clearExisting) { // ------------------------------------------------------------------------ /** - * Creates a new class that is not part of the classpath that the current JVM uses, and - * instantiates it. + * A new object and the corresponding ClassLoader for that object, as returned by + * {@link #createObjectFromNewClassLoader(ClassLoader)}. + */ + public static final class ObjectAndClassLoader { + + private final Serializable object; + private final ClassLoader classLoader; + + private ObjectAndClassLoader(Serializable object, ClassLoader classLoader) { + this.object = object; + this.classLoader = classLoader; + } + + public ClassLoader getClassLoader() { + return classLoader; + } + + public Serializable getObject() { + return object; + } + } + + /** + * Creates a new ClassLoader and a new class inside that ClassLoader. + * This is useful when unit testing the class loading behavior of code, and needing a class that + * is outside the system class path. * - *

This method uses {@link #createClassNotInClassPath(ClassLoader)} to define the new class. + *

This method behaves like {@code createObjectFromNewClassLoader(ClassLoader.getSystemClassLoader())}. + */ + public static ObjectAndClassLoader createObjectFromNewClassLoader() { + return createObjectFromNewClassLoader(ClassLoader.getSystemClassLoader()); + } + + /** + * Creates a new ClassLoader and a new class inside that ClassLoader. + * This is useful when unit testing the class loading behavior of code, and needing a class that + * is outside the system class path. * - * @param targetClassLoader The class loader to attach the class to - * @return The object instantiated from the newly defined class. + *

NOTE: Even though this method may throw IOExceptions, we do not declare those and rather + * wrap them in Runtime Exceptions. While this is generally discouraged, we do this here because it + * is merely a test utility and not production code, and it makes it easier to use this method + * during the initialization of variables and especially static variables. + * + * @param parentClassLoader The parent class loader used for the newly created class loader. */ - public static Serializable createObjectForClassNotInClassPath(ClassLoader targetClassLoader) { + public static ObjectAndClassLoader createObjectFromNewClassLoader(@Nullable ClassLoader parentClassLoader) { + final String testClassName = "org.apache.flink.TestSerializable"; + final String testClassFile = testClassName.replace('.', '/') + ".class"; + + final Path classDirPath = new File(System.getProperty("java.io.tmpdir"), UUID.randomUUID().toString()).toPath(); + final Path classFilePath = classDirPath.resolve(testClassFile); + + URLClassLoader classLoader = null; try { - Class clazz = createClassNotInClassPath(targetClassLoader); - return clazz.newInstance(); + Files.createDirectories(classFilePath.getParent()); + writeClassFile(classFilePath); + + final URL[] classPath = new URL[] {classDirPath.toUri().toURL()}; + classLoader = parentClassLoader == null ? + new URLClassLoader(classPath) : + new URLClassLoader(classPath, parentClassLoader); + + final Class clazz = classLoader.loadClass(testClassName); + final Serializable object = clazz.asSubclass(Serializable.class).newInstance(); + + return new ObjectAndClassLoader(object, classLoader); } catch (Exception e) { - throw new AssertionError("test setup broken", e); + throw new RuntimeException("Cannot create test class outside system class path", e); + } + finally { + // we clean up eagerly, because it is fine to delete the class file once the class is loaded + // and we have no later life cycle hook here to do the cleanup + tryClose(classLoader); + tryDeleteDirectoryRecursively(classDirPath); } } - /** - * Creates a new class that is not part of the classpath that the current JVM uses. - * The class is ad-hoc defined and attached to the given ClassLoader. - * - * @param targetClassLoader The class loader to attach the class to - * @return The newly defined class - */ - public static Class createClassNotInClassPath(ClassLoader targetClassLoader) { + private static void writeClassFile(Path path) throws IOException { + // this is the byte code of the following simple class: + // ---------------------------------------------------------------------- + // package org.apache.flink; + // + // public class TestSerializable implements java.io.Serializable {} + // ---------------------------------------------------------------------- + final byte[] classData = {-54, -2, -70, -66, 0, 0, 0, 51, 0, 65, 10, 0, 15, 0, 43, 7, 0, 44, 10, 0, 2, 0, 43, 10, 0, 2, 0, 45, 9, 0, 7, 0, 46, 10, 0, 15, 0, 47, 7, 0, 48, 7, 0, 49, 10, 0, 8, 0, 43, 8, 0, 50, 10, 0, 8, 0, 51, 10, 0, 8, 0, 52, 10, 0, 8, 0, 53, 10, @@ -230,37 +298,8 @@ public static Class createClassNotInClassPath(ClassLoade -80, 0, 0, 0, 2, 0, 26, 0, 0, 0, 6, 0, 1, 0, 0, 0, 51, 0, 27, 0, 0, 0, 12, 0, 1, 0, 0, 0, 28, 0, 28, 0, 29, 0, 0, 0, 1, 0, 41, 0, 0, 0, 2, 0, 42}; - try { - // define a class into the classloader - Class clazz = getUnsafe().defineClass( - "org.apache.flink.TestSerializable", - classData, 0, classData.length, - targetClassLoader, - new ProtectionDomain(new CodeSource(null, (Certificate[]) null), new Permissions())); - - return clazz.asSubclass(Serializable.class); - } - catch (Exception e) { - throw new AssertionError("test setup broken", e); - } - } - - @SuppressWarnings("restriction") - private static sun.misc.Unsafe getUnsafe() { - try { - Field unsafeField = sun.misc.Unsafe.class.getDeclaredField("theUnsafe"); - unsafeField.setAccessible(true); - return (sun.misc.Unsafe) unsafeField.get(null); - } catch (SecurityException e) { - throw new RuntimeException("Could not access the sun.misc.Unsafe handle, permission denied by security manager.", e); - } catch (NoSuchFieldException e) { - throw new RuntimeException("The static handle field in sun.misc.Unsafe was not found."); - } catch (IllegalArgumentException e) { - throw new RuntimeException("Bug: Illegal argument reflection access for static field.", e); - } catch (IllegalAccessException e) { - throw new RuntimeException("Access to sun.misc.Unsafe is forbidden by the runtime.", e); - } catch (Throwable t) { - throw new RuntimeException("Unclassified error while trying to access the sun.misc.Unsafe handle.", t); + try (OutputStream out = Files.newOutputStream(path, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE)) { + out.write(classData); } } @@ -285,4 +324,38 @@ public static boolean containsCause(Throwable throwable, Class deletingVisitor = new SimpleFileVisitor() { + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + Files.delete(file); + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException { + Files.delete(dir); + return FileVisitResult.CONTINUE; + } + }; + + try { + Files.walkFileTree(directory, deletingVisitor); + } + catch (Exception ignored) {} + } } diff --git a/flink-test-utils-parent/flink-test-utils-junit/src/test/java/org/apache/flink/core/testutils/CommonTestUtilsTest.java b/flink-test-utils-parent/flink-test-utils-junit/src/test/java/org/apache/flink/core/testutils/CommonTestUtilsTest.java new file mode 100644 index 0000000000000..774f77e89baeb --- /dev/null +++ b/flink-test-utils-parent/flink-test-utils-junit/src/test/java/org/apache/flink/core/testutils/CommonTestUtilsTest.java @@ -0,0 +1,53 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.core.testutils; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.fail; + +/** + * Tests for the {@link CommonTestUtils}. + */ +public class CommonTestUtilsTest { + + @Test + public void testObjectFromNewClassLoaderObject() throws Exception { + final CommonTestUtils.ObjectAndClassLoader objectAndClassLoader = CommonTestUtils.createObjectFromNewClassLoader(); + final Object o = objectAndClassLoader.getObject(); + + assertNotEquals(ClassLoader.getSystemClassLoader(), o.getClass().getClassLoader()); + + try { + Class.forName(o.getClass().getName()); + fail("should not be able to load class from the system class loader"); + } + catch (ClassNotFoundException ignored) {} + } + + @Test + public void testObjectFromNewClassLoaderClassLoaders() throws Exception { + final CommonTestUtils.ObjectAndClassLoader objectAndClassLoader = CommonTestUtils.createObjectFromNewClassLoader(); + + assertNotEquals(ClassLoader.getSystemClassLoader(), objectAndClassLoader.getClassLoader()); + assertEquals(ClassLoader.getSystemClassLoader(), objectAndClassLoader.getClassLoader().getParent()); + } +}