RFR: 8335896: Source launcher should set TCCL

[Jaikiran Pai]

On Mon, 15 Jul 2024 16:19:20 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> src/jdk.compiler/share/classes/com/sun/tools/javac/launcher/SourceLauncher.java line 204:
>> 
>>> 202:             ClassLoader loader = context.newClassLoaderFor(parentLoader, firstClassName);
>>> 203:             firstClass = Class.forName(firstClassName, false, loader);
>>> 204:             Thread.currentThread().setContextClassLoader(loader);
>> 
>> Hello Christian, a typical pattern for such Thread context classloader switches is to reset the context classloader back to the previous one, once the "work" is done. That way it doesn't influence the code after the completion of the work.
>> 
>> So something like the following might be more safer:
>> 
>> 
>> ClassLoader loader;
>> try {
>>     loader = context.newClassLoaderFor(parentLoader, firstClassName);
>>     firstClass = Class.forName(firstClassName, false, loader);
>> } catch (ClassNotFoundException e) {
>>     throw new Fault(Errors.CantFindClass(firstClassName));
>> }
>> final ClassLoader prevTCCL = Thread.currentThread().getContextClassLoader();
>> Thread.currentThread().setContextClassLoader(loader);
>> try {
>>     Method mainMethod = MethodFinder.findMainMethod(firstClass);
>>     ... // rest of the existing code
>>     return mainClass;
>> } finally {
>>     Thread.currentThread().setContextClassLoader(prevTCCL);
>> }
>
> @jaikiran The PR is the equivalent of System.initPhase3 where it sets the TCCL for the main thread. With the source code launcher, the defining loader of the source "main" is a child of the system (usually application) class loader. So it's not meant to be temporary setting.

Thank you Alan for that detail.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20097#discussion_r1678101644