[GitHub] [flink] zentol commented on a change in pull request #11303: [Flink-16245] Decoupling user classloader from context classloader.

[GitBox <gi...@apache.org>]

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_r387551180
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/FlinkUserCodeClassLoaders.java
 ##########
 @@ -82,4 +91,78 @@ 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.
+	 *
+	 * @param <T> the classloader type that also needs to be closeable.
+	 */
+	private static class SafetyNetWrapperClassLoader<T extends ClassLoader & Closeable> extends CloseableClassLoader
+			implements Closeable {
+		private static final Logger LOG = LoggerFactory.getLogger(SafetyNetWrapperClassLoader.class);
+
+		private T inner;
+
+		SafetyNetWrapperClassLoader(T inner) {
+			super(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);
+			}
+
+			synchronized (getClassLoadingLock(name)) {
 
 Review comment:
   why aren't we just forwarding the call to the inner class?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services