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

[David Holmes]

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?

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
>>>
>