On 9/8/16 00:38, David Holmes wrote:
On 8/09/2016 5:33 PM, serguei.spit...@oracle.com wrote:
Hi David,

On 9/8/16 00:22, David Holmes wrote:
Hi Serguei,

On 8/09/2016 5:27 AM, serguei.spit...@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



Reply via email to