azagrebin commented on a change in pull request #12446:
URL: https://github.com/apache/flink/pull/12446#discussion_r435739183
##########
File path:
flink-core/src/main/java/org/apache/flink/util/ClassLoaderWithErrorHandler.java
##########
@@ -18,47 +18,76 @@
package org.apache.flink.util;
+import java.io.IOException;
+import java.io.InputStream;
import java.net.URL;
import java.net.URLClassLoader;
+import java.util.Enumeration;
import java.util.function.Consumer;
/**
- * This class loader accepts a custom handler if an exception occurs in {@link
#loadClass(String, boolean)}.
+ * This {@link URLClassLoader} decorator accepts a custom exception handler.
+ *
+ * <p>The handler is called if an exception occurs in the {@link
#loadClass(String, boolean)} of inner class loader.
*/
-public abstract class ClassLoaderWithErrorHandler extends URLClassLoader {
+public class ClassLoaderWithErrorHandler extends URLClassLoader {
public static final Consumer<Throwable> EMPTY_EXCEPTION_HANDLER =
classLoadingException -> {};
+ private static final URL[] EMPTY_URL_ARRAY = {};
+ private final URLClassLoader inner;
private final Consumer<Throwable> classLoadingExceptionHandler;
- protected ClassLoaderWithErrorHandler(URL[] urls, ClassLoader parent) {
- this(urls, parent, EMPTY_EXCEPTION_HANDLER);
- }
-
- protected ClassLoaderWithErrorHandler(
- URL[] urls,
- ClassLoader parent,
+ public ClassLoaderWithErrorHandler(
+ URLClassLoader inner,
Consumer<Throwable> classLoadingExceptionHandler) {
- super(urls, parent);
+ super(EMPTY_URL_ARRAY, inner.getParent());
+ this.inner = inner;
this.classLoadingExceptionHandler =
classLoadingExceptionHandler;
}
- @SuppressWarnings("FinalMethod")
@Override
- protected final Class<?> loadClass(String name, boolean resolve) throws
ClassNotFoundException {
- try {
- return loadClassWithoutExceptionHandling(name, resolve);
- } catch (Throwable classLoadingException) {
-
classLoadingExceptionHandler.accept(classLoadingException);
- throw classLoadingException;
+ protected Class<?> loadClass(final String name, final boolean resolve)
throws ClassNotFoundException {
+ synchronized (getClassLoadingLock(name)) {
+ final Class<?> loadedClass = findLoadedClass(name);
+ if (loadedClass != null) {
+ return resolveIfNeeded(resolve, loadedClass);
+ }
+
+ try {
+ return resolveIfNeeded(resolve,
inner.loadClass(name));
+ } catch (Throwable classLoadingException) {
+
classLoadingExceptionHandler.accept(classLoadingException);
+ throw classLoadingException;
+ }
}
}
- /**
- * Same as {@link #loadClass(String, boolean)} but without exception
handling.
- *
- * <p>Extending concrete class loaders should implement this instead of
{@link #loadClass(String, boolean)}.
- */
- protected Class<?> loadClassWithoutExceptionHandling(String name,
boolean resolve) throws ClassNotFoundException {
- return super.loadClass(name, resolve);
+ private Class<?> resolveIfNeeded(final boolean resolve, final Class<?>
loadedClass) {
+ if (resolve) {
+ resolveClass(loadedClass);
+ }
+
+ return loadedClass;
+ }
+
+ @Override
+ public URL getResource(final String name) {
+ return inner.getResource(name);
+ }
+
+ @Override
+ public Enumeration<URL> getResources(final String name) throws
IOException {
+ return inner.getResources(name);
+ }
+
+ @Override
+ public InputStream getResourceAsStream(String name) {
+ return inner.getResourceAsStream(name);
+ }
+
+ @Override
+ public void close() throws IOException {
+ super.close();
+ inner.close();
Review comment:
Is it not the case that we firstly construct `inner` outside of the
decorator and then the decorator? Not sure whether this matters in this case.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
