On Sun, 17 Apr 2022 12:39:24 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

> I had a look at the referenced 
> [JDK-4006245](https://bugs.openjdk.java.net/browse/JDK-4006245). The 
> previous/existing reference cleaning logic seems to have been added mainly to 
> `null` the reference to the user/application class instance represented by 
> the `Runnable` task. Of course, other references too were being cleared. The 
> description in the linked JBS seems to suggest that the `Thread` instance 
> wasn't being GCed sooner. That was more than 2 decades back. I don't have 
> enough knowledge of this area to know if this is still an issue in recent GC 
> implementations.

The comment on that method dates from early JDK releases with the Classic VM 
and its conservative GC. It also predates the java.lang.ref API.  The question 
that I think you are asking is if you create a Thread with a runnable task, let 
it terminate but keep the reference to the Thread forever then what does it 
keep alive. We can think about more but so far we haven't seen any issues or 
tests fail with the reference to the runnable task.

> src/java.base/share/classes/java/lang/Thread.java line 2414:
> 
>> 2412:             sm.checkPermission(new 
>> RuntimePermission("setContextClassLoader"));
>> 2413:         }
>> 2414:         if (!isSupportedClassLoader(contextClassLoader)) {
> 
> For a few moments, I thought this was a typo and should instead have been:
> 
> if (!isSupportedClassLoader(cl)) {
> 
> but then thinking a bit more, I realized what's being done here. Essentially, 
> this checks whether the _current_ context classloader  is set to "not allowed 
> to change/set context classloader" (represented by 
> `Constants.NOT_SUPPORTED_CLASSLOADER`).
> 
> Do you think this line deserves some short code comment that it intentionally 
> passes the current contextClassLoader (and not the one passed to the method) 
> to `isSupportedClassLoader`?

I think the comment on isSupportedClassLoader is readable so I think it's okay.

-------------

PR: https://git.openjdk.java.net/jdk/pull/8166

Reply via email to