AHeise 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_r393616067
########## 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: I thought that's what you meant with fail early. If you look into https://issues.apache.org/jira/browse/FLINK-11205, you can see the main motivation for this PR. Here Stephan wrote > Some thoughts on how to approach this are: > ... > As a final safety-net, the TMs kill/restart themselves when the metaspace blows up FLINK-16225 > ... > Joey Echeverria A generic mechanism to prevent leaks through ClassLoader caching (as in Apache Commons Logging) would be FLINK-16245 (use a delegating class loader where we drop the reference to the real one when closing it). Btw, this probably answers your initial point in this thread: throwing ISE it is. Now, I just need to figure out how to make sure that this exception will actually let TM fail instead of any later job. ---------------------------------------------------------------- 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: us...@infra.apache.org With regards, Apache Git Services