StephanEwen commented on a change in pull request #11303: [FLINK-16245]
Decoupling user classloader from context classloader.
URL: https://github.com/apache/flink/pull/11303#discussion_r396122590
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/FlinkUserCodeClassLoaders.java
##########
@@ -82,4 +89,61 @@ public static ResolveOrder fromString(String resolveOrder) {
super(urls, parent);
}
}
+
+ /**
+ * Ensures that holding a reference on the context class loader
outliving the scope of user code does not prevent
+ * the user classloader to be garbage collected (FLINK-16245).
+ *
+ * <p>This classloader delegates to the actual user classloader. Upon
{@link #close()}, the delegate is nulled
+ * and can be garbage collected. Additional class resolution will be
resolved solely through the bootstrap
+ * classloader and most likely result in ClassNotFound exceptions.
+ */
+ private static class SafetyNetWrapperClassLoader extends URLClassLoader
+ implements Closeable {
+ private static final Logger LOG =
LoggerFactory.getLogger(SafetyNetWrapperClassLoader.class);
+
+ private FlinkUserCodeClassLoader inner;
+
+ SafetyNetWrapperClassLoader(FlinkUserCodeClassLoader inner) {
+ super(new URL[0], null);
+ this.inner = inner;
+ }
+
+ @Override
+ public void close() {
+ if (inner != null) {
+ try {
+ inner.close();
+ } catch (IOException e) {
+ LOG.warn("Could not close user
classloader", e);
+ }
+ }
+ inner = null;
+ }
+
+ @Override
+ protected Class<?> loadClass(String name, boolean resolve)
throws ClassNotFoundException {
+ if (inner == null) {
+ return super.loadClass(name, resolve);
Review comment:
My thought would be mainly understandable behavior. The change targets cases
of caching beyond execution of slot related tasks, such as in logging
frameworks. Users would really wonder why java core classes could not be loaded
any more.
Aside from that, if the logging framwork only accesses core classes, this
change would break nothing, while forbidding to load all classes might break
some setups.
Related: Might make sense to wrap the `ClassNotFoundException` and enhance
the error message with "Flink user code classloader is shut down".
----------------------------------------------------------------
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]
With regards,
Apache Git Services
