zentol 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_r393181657
 
 

 ##########
 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:
   Why are we not failing immediately in this case? This introduces subtle 
inconsistent behaviors, that to fully grasp require a good understanding of how 
class-loading works in the JVM which really shows in the added 
`testClosingOfClassloader` test. I would wager that barely anyone would be able 
to explain why the class cannot be loaded at the end of the test.

----------------------------------------------------------------
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

Reply via email to