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

[Rafael Winterhalter]

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.

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.

Thanks for considering this use case!
Best regards, Rafael

2018-04-15 8:23 GMT+02:00 mandy chung <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/
>
> Mandy
> [1] 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/20180416/4bb7dad5/attachment-0001.html>