Review Request JDK-8200559: Java agents doing instrumentation need a means to define auxiliary classes
Rafael Winterhalter
rafael.wth at gmail.com
Wed Jul 4 19:08:04 UTC 2018
Hi Mandy and Alan,
I very much understand your points of view that the Java instrumentation
API should retain its original intended scope and that every problem should
be solved at its own time. I do however claim that the proposed API does
not even solve this problem of auxiliary classes for the general case. By
bringing up several examples for why I want to suggest a
package-independent injection mechanism, this argument was maybe lost
between the lines. Allow me to repeat it in a possibly clearer manner:
Consider a case where an instrumentation is not isolated to a single class
but involves the classes foo.Bar and qux.Baz which are both transformed. To
implement the instrumentation some code is added to a method of foo.Bar
which then invokes the method qux.Baz::baz(Object) which is also
transformed. To apply the instrumentation, an auxiliary class needs to be
created which transports some state from foo.Bar to qux.Baz as the method
argument. For this to work, the code that is added to qux.Baz::baz(Object)
checks and casts the argument to the known auxiliary class.
Using the suggested API, it is very difficult to apply an transformation as
it is not clear if the auxiliary class should live in the foo or the qux
package as it is not controlled by the Java agent which of the foo.Bar and
qux.Baz classes is loaded first. To solve this, one would need to prepare
two instrumentations with an auxiliary class in the foo or the qux package
to define the class according to the load order. This problem explodes into
multidimensional complexity as more classes are involved in an
instrumentation.
This example might seem artificial but I it makes perfect sense when for
example instrumenting actors in an actor framework such as Akka where a new
message is added at runtime and handling is added to two actor classes. I
have also encountered similar instrumentation circles in I/O processing
frameworks.
And while an instrumentation might start isolated to transforming a single
class, there might be a requirement to evolve it later. Given the proposed
API, such evolutions are now difficult to implement as defining auxiliary
classes requires to consider class loading order.
This is my argument for this API not being suited for instrumentation
purposes and why I would favor an Instrumentation::defineClass API instead.
It is simply to difficult to find a good limiting factor; one could of
course consider to allow class definitions in the same class loader or
module but since class loaders and modules can stand in exporting
relationships to one another the problem with unpredictable load order
would occur again.
Beyond that I still claim that beyond the use case of auxiliary classes,
the following facts justify an introduction of Instrumentation::defineClass
which all apply to "tool agents" towards which the Instrumentation API is
targeted:
1. The need to inject dispatchers into specific class loaders to allow
cross-class loader communication. This is typically the bootstrap class
loader which is already accessible via
Instrumentation::appendToBootstrapSearchPath but this might be too specific.
2. The factual possibility of an owner of an instrumentation instance to
inject classes into any package without using internal API simply by
"pseudo transforming" a class that resides in the desired package.
3. The history of agents being developed using sun.misc.Unsafe::defineClass
for many years what makes migration to a much different API unlikely if
this involves heavy costs.
4. Retaining some equivalence to native agents where an API for defining
classes is available via JNI. It would be a shame if JVMTI is favored over
the Java agent API only for this.
I really hope you take this concern into consideration, To strengthen my
argument, please also considered that many regard me to be one of the
leading experts for JVM agents (due to my library Byte Buddy that is often
used for Java agents) and I have worked with a multitude of Java agent
vendors whose concerns I am also voicing here. Currently, none of the
vendors I regularly talk to is considering to use the suggested API whereas
most plan to simply migrate to jdk.internal.misc.Unsafe what is further
fostered by no alternative being offered in Java 11.
I also understand that it is probably too late to make an API decision at
this point. However, due to this important use case for
sun.misc.Unsafe::defineClass not being currently covered, it want to
suggest to reintroduce the latter method to Java 11 to avoid a further
spread of internal API usage by forcing people into
jdk.internal.misc.Unsafe where many will grow comfortable and not even
consider future APIs. The migration to jdk.internal.misc.Unsafe is also
what I observe being used for EA builds at this point so this is a partial
reality already.
Thank you for hearing me out, I really hope I can change your mind on this
issue and add Instrumentation::defineClass.
best regards, Rafael
2018-07-02 19:17 GMT+02:00 mandy chung <mandy.chung at oracle.com>:
> My proposal of ClassDefiner API allows the java agent to define auxiliary
> classes in the same runtime package of the class being instrumented. You
> raised other use cases that are not addressed by this proposal. As Alan
> replied, the ability to define any arbitrary class would be an attractive
> nuisance and we think Instrumentation.defineClass isn't the right API to
> add.
>
> I think the proposed ClassDefiner API is useful for the specific use case
> (define auxiliary classes in the runtime package of the class being
> instrumented). I hold it off and so didn't make 11. For the other use
> cases, perhaps we should create JBS issues for further investigation.
>
> Mandy
>
> On 7/2/18 1:41 AM, Rafael Winterhalter wrote:
>
>> Hi,
>>
>> I was wondering if a solution for this problem is still planned for JDK
>> 11 giving the beginning ramp down.
>>
>> With removing sun.misc.Unsafe::defineClass, Java agents only have an
>> option to use jdk.internal.misc.Unsafe::defineClass for the use-cases
>> that I described.
>>
>> I think it would be a missed opportunity not to offer an alternative as
>> of JDK 11 as a second migration would make it even less likely that agents
>> would avoid unsafe API.
>>
>> Thanks for the information,
>> best regards, Rafael
>>
>> mandy chung <mandy.chung at oracle.com <mailto:mandy.chung at oracle.com>>
>> schrieb am So., 15. Apr. 2018, 08:23:
>>
>> 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/20180704/b95ddd0a/attachment.html>
More information about the serviceability-dev
mailing list