[jira] [Comment Edited] (FLINK-16245) Use a delegating classloader as the user code classloader to prevent class leaks.

["Arvid Heise (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/FLINK-16245?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17046933#comment-17046933 ] 

Arvid Heise edited comment on FLINK-16245 at 2/27/20 7:38 PM:
--------------------------------------------------------------

I investigated a bit and I'm convinced that this fix will not help to avoid FLINK-11205. It would only help in cases where context classloader would have been cached for some reason (and this sounds very esoteric).

Let's assume we currently have `UserCL <-> SomeClass <- JMX`. Now if we add a naive delegation, it looks like `DelegateCL -> UserCL <-> SomeClass <- JMX`, since the returned class would still be actually loaded through UserCL. Thus, we would have the same situation as now.

Now, let's say we find a fancy way to do `DelegateCL -> UserCL -> SomeClass -> DelegateCL`. Chances are high that we would still end up with `SomeClass <-> DelegateCL`, since `ClassLoader#findLoadedClass` needs to store the information to abide its contract.

{color:#629755}Returns the class with the given {color}{color:#77b767}<a href="#name">{color}{color:#629755}binary name{color}{color:#77b767}</a> {color}{color:#629755}if this
{color}{color:#629755}* loader has been recorded by the Java virtual machine as an initiating
{color}{color:#629755}* loader of a class with that {color}{color:#77b767}<a href="#name">{color}{color:#629755}binary name{color}{color:#77b767}</a>{color}{color:#629755}. Otherwise
{color}{color:#629755}* {color}{color:#77b767}<tt>{color}{color:#629755}null{color}{color:#77b767}</tt> {color}{color:#629755}is returned.{color}

Most likely it will store the loaded class in its hidden field "classes". So we effectively end up where we started with `DelegateCL <-> SomeClass <- JMX`.

Lastly, let's even assume we manage to avoid that. We'd end up with some class `SomeClass -> DelegateCL -> null`. Now what if any instance is still used (JMX), are referenced classes still loaded? Then we would have gained not much. If not, it cannot load the classes anymore.

---

TL;DR I cannot find a scenario where this indirection works for the motivating use case.

All solutions to proper class unloading always point to getting rid of all instances and that's where we should focus our effort. Having a shutdown hook for user code classloader would certainly help.


was (Author: arvid heise):
I investigated a bit and I'm convinced that this fix will not help to avoid FLINK-11205. It would only help in cases where context classloader would have been cached for some reason (and this sounds very esoteric).

Let's assume we currently have UserCL <-> SomeClass <- JMX. Now if we add a naive delegation, it looks like DelegateCL -> UserCL <-> SomeClass <- JMX, since the returned class would still be actually loaded through UserCL. Thus, we would have the same situation as now.

Now, let's say we find a fancy way to do DelegateCL -> UserCL -> SomeClass -> DelegateCL. Chances are high that we would still end up with SomeClass <-> DelegateCL, since ClassLoader#findLoadedClass needs to store the information to abide its contract.

{color:#629755}Returns the class with the given {color}{color:#77b767}<a href="#name">{color}{color:#629755}binary name{color}{color:#77b767}</a> {color}{color:#629755}if this
{color}{color:#629755}* loader has been recorded by the Java virtual machine as an initiating
{color}{color:#629755}* loader of a class with that {color}{color:#77b767}<a href="#name">{color}{color:#629755}binary name{color}{color:#77b767}</a>{color}{color:#629755}. Otherwise
{color}{color:#629755}* {color}{color:#77b767}<tt>{color}{color:#629755}null{color}{color:#77b767}</tt> {color}{color:#629755}is returned.{color}

Most likely it will store the loaded class in its hidden field "classes". So we effectively end up where we started with DelegateCL <-> SomeClass <- JMX.

Lastly, let's even assume we manage to avoid that. We'd end up with some class SomeClass -> DelegateCL -> null. Now what if any instance is still used (JMX), are referenced classes still loaded? Then we would have gained not much. If not, it cannot load the classes anymore.

---

TL;DR I cannot find a scenario where this indirection works for the motivating use case.

All solutions to proper class unloading always point to getting rid of all instances and that's where we should focus our effort. Having a shutdown hook for user code classloader would certainly help.

> Use a delegating classloader as the user code classloader to prevent class leaks.
> ---------------------------------------------------------------------------------
>
>                 Key: FLINK-16245
>                 URL: https://issues.apache.org/jira/browse/FLINK-16245
>             Project: Flink
>          Issue Type: Improvement
>          Components: Runtime / Task
>            Reporter: Stephan Ewen
>            Priority: Critical
>              Labels: usability
>             Fix For: 1.11.0
>
>
> As reported in FLINK-11205, a reference to the user-code ClassLoader can be held by some libraries, causing class leaks.
> One way to circumvent this class leak is if the ClassLoader that we set as the user-code ClassLoader is a delegating ClassLoader to the real class loader, and when closing the user code ClassLoader we null out the reference.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)