Review Request JDK-8200559: Java agents doing instrumentation need a means to define auxiliary classes

[mandy chung]

Hi Rafael,

I see that mocking/proxying/testing framework should be looked at 
separately since its requirements and approaches can be different than 
tool agents.

On 4/17/18 5:06 AM, Rafael Winterhalter wrote:
> Hei Mandy,
>
> I have looked into several Java agents that I have worked on and for 
> many of them, this API does unfortunately not supply sufficient 
> access. I would therefore still prefer a method 
> Instrumentation::defineClass.
>
> The problem is that some agents need to define classes in other 
> packages then in that of the instrumented class. For example, I might 
> need to enhance a library that defines a set of callback classes in 
> package A. All these classes share a common super class with a 
> package-private constructor. I want to instrument some class in 
> package B to use a callback that the library does not supply and need 
> to add a new callback class to A. This is not possible using the 
> current API.
>

Are these callback classes made available statically?  or just 
dynamically defining additional class as needed?  Is Lookup::defineClass 
an alternative if you get a hold of common super class in A?

> I could however achieve do so by calling Instrumentation::retransform 
> on one of the classes in A after registering a class file transformer. 
> Once the retransformation is triggered, I can now define a class in A. 
> Of course this is inefficient and I would rather open the 
> jdk.internal.misc module and use the "old" API instead.
>
> For this reason, I argue that this rather restrained API is not 
> convenient while it does not add anything to security. Also, for the 
> use case of Mockito, this would neither be sufficient as Mockito 
> sometimes redefines classes and sometimes adds a subclass without 
> retransforming. We would rather have direct access to class definition 
> once we are already running with the privileges of a Java agent.
>
> I would therefore suggest to add a method:
>
> interface Instrumentation {
>   Class<?> defineClass(byte[] bytes, ProtectionDomain pd);
> }
>
> which can be implemented simply by delegating to jdk.internal.misc.Unsafe.
>
> On a side note. Does JavaLangAccess::defineClass work with the 
> bootstrap class loader? I have not tried it but I always thought it 
> was just an access layer for the class loader API that cannot access 
> the null value.
>

The JVM entry point does allow null loader.

Mandy

> Thanks for considering this use case!
> Best regards, Rafael
>
> 2018-04-15 8:23 GMT+02:00 mandy chung <mandy.chung at oracle.com 
> <mailto:mandy.chung at oracle.com>>:
>
>     Background:
>
>     Java agents support both load time and dynamic instrumentation.  
>     At load time,
>     the agent's ClassFileTransformer is invoked to transform class
>     bytes.  There is
>     no Class objects at this time.  Dynamic instrumentation is when
>     redefineClasses
>     or retransformClasses is used to redefine an existing loaded
>     class.  The
>     ClassFileTransformer is invoked with class bytes where the Class
>     object is present.
>
>     Java agent doing instrumentation needs a means to define auxiliary
>     classes
>     that are visible and accessible to the instrumented class. 
>     Existing agents
>     have been using sun.misc.Unsafe::defineClass to define aux classes
>     directly
>     or accessing protected ClassLoader::defineClass method with
>     setAccessible to
>     suppress the language access check (see [1] where this issue was
>     brought up).
>
>     Instrumentation::appendToBootstrapClassLoaderSearch and
>     appendToSystemClassLoaderSearch
>     APIs are existing means to supply additional classes.  It's too
>     limited
>     for example it can't inject a class in the same runtime package as
>     the class
>     being transformed.
>
>     Proposal:
>
>     This proposes to add a new ClassFileTransformer.transform method
>     taking additional ClassDefiner parameter.  A transformer can
>     define additional
>     classes during the transformation process, i.e.
>     when ClassFileTransformer::transform is invoked. Some details:
>
>     1. ClassDefiner::defineClass defines a class in the same runtime
>     package
>        as the class being transformed.
>     2. The class is defined in the same thread as the transformers are
>     being
>        invoked.   ClassDefiner::defineClass returns Class object directly
>        before the transformed class is defined.
>     3. No transformation is applied to classes defined by
>     ClassDefiner::defineClass.
>
>     The first prototype we did is to collect the auxiliary classes and
>     define
>     them  until all transformers are invoked and have these aux
>     classes to go
>     through the transformation pipeline.  Several complicated issues
>     would
>     need to be resolved for example timing whether the auxiliary
>     classes should
>     be defined before the transformed class (otherwise a potential
>     race where
>     some other thread references the transformed class and cause the
>     code to
>     execute that in turn reference the auxiliary classes. The current
>     implementation has a native reentrancy check that ensure one class
>     is being
>     transformed to avoid potential circularity issues.  This may need
>     JVM TI
>     support to be reliable.
>
>     This proposal would allow java agents to migrate from internal API
>     and ClassDefiner to be enhanced in the future.
>
>     Webrev:
>     http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8200559/webrev.00/
>     <http://cr.openjdk.java.net/%7Emchung/jdk11/webrevs/8200559/webrev.00/>
>
>     Mandy
>     [1]
>     http://mail.openjdk.java.net/pipermail/jdk-dev/2018-January/000405.html
>     <http://mail.openjdk.java.net/pipermail/jdk-dev/2018-January/000405.html>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180417/32745e1a/attachment.html>