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:44 PM, serguei.spitsyn at oracle.com wrote:
> 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.

Confused again - the main class should not be loaded by the built-in App 
loader if a custom system loader was specified!

David

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