RFR: 8160950 Agent JAR added to app class loader rather than system class loader when running with -Djava.system.class.loader

[serguei.spitsyn at oracle.com]

On 9/8/16 00:38, David Holmes wrote:
> On 8/09/2016 5:33 PM, serguei.spitsyn at oracle.com wrote:
>> Hi David,
>>
>> On 9/8/16 00:22, David Holmes wrote:
>>> Hi Serguei,
>>>
>>> On 8/09/2016 5:27 AM, serguei.spitsyn at oracle.com wrote:
>>>> Please, review the fix for:
>>>>   https://bugs.openjdk.java.net/browse/JDK-8160950
>>>>
>>>> Webrev:
>>>>
>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/jdk/8160950-JLI-WLS.jdk2/ 
>>>>
>>>>
>>>>
>>>>
>>>> Summary:
>>>>   When running a java application with the options
>>>> `-javaagent:myagent.jar -Djava.system.classloader=MyClassLoader`
>>>>   then the myagent.jar is added to the application class loader rather
>>>> than the custom system class loader.
>>>
>>> I'm confused, the "system" class loader is the "application" class
>>> loader.
>>
>> Yes, the App class loader is the default (or builtin) system class 
>> loader.
>> The option -Djava.system.classloader=MyClassLoader` makes the custom
>> class loader MyClassLoader to become the (custom) system class loader.
>
> IIRC we have a new Platform loader which has the boot-loader as the 
> parent. During early initialization we create the built-in App loader 
> (with the Platform loader as parent) to be the "system" loader, but 
> that can be replaced later by a user-specified system loader. So the 
> bug is that it got associated with the temporary "system" loader, not 
> the final intended "system" loader. Is that right?

Yes, that is right.
I also was confused by finding that the java main class is still loaded 
by the built-in App loader.
But the Agent class is loaded by the custom system class loader.
It is why the test has some tricks.

Thanks,
Serguei

>
> Thanks,
> David
>
>> Thanks,
>> Serguei
>>
>>>
>>> David
>>> -----
>>>
>>>>   The problem was that the libinstrument Agent_OnLoad did not 
>>>> invoke the
>>>>   custom system class loader method 
>>>> appendToClassPathForInstrumentation.
>>>>   Instead, it added the agent's jar path into the system property
>>>> "java.class.path".
>>>>
>>>>   The solution is to save the jar file path in the global structure 
>>>> and
>>>> later use it to
>>>>   invoke the custom system class loader's method
>>>> appendToClassPathForInstrumentation.
>>>>
>>>>   This breaks a compatibility in the rare case if a custom system 
>>>> class
>>>> loader
>>>>   does not define the method appendToClassPathForInstrumentation.
>>>>   The compatibility risk is low because using a custom system class
>>>> loader is very rare
>>>>   (little known feature). In addition, any custom class loader used in
>>>> environments with
>>>>   tools that load agents into running VMs already define this method.
>>>>   In the unlikely event that there is a custom system class loader 
>>>> that
>>>> does not define this
>>>>   method, and it is used with a java agent specified on command line,
>>>> then the error will be
>>>>   clear as the VM will refuse to start. It is trivially fixed by
>>>> defining the method.
>>>>
>>>>   The previous version of the fix was preliminary reviewed by Alan and
>>>> Mandy.
>>>>
>>>> Testing:
>>>>   Ran the jtreg java.lang.instrument test and new unit test
>>>>
>>>> Thanks,
>>>>
>>>> Serguei
>>>>
>>