[ 
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:47 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.

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

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)

Reply via email to