https://bugs.openjdk.java.net/browse/JDK-8201784
This is the correct JBS issue (Sorry I cut-n-paste the wrong link).
Mandy
On 4/18/18 3:46 PM, mandy chung wrote:
Hi Rafael,
I think it's best to separate the testing requirement from java agents
doing instrumentation that may run in production environment.
I have created a JBS issue to track the testing mode idea that would
require more discussion and investigation:
https://bugs.openjdk.java.net/browse/JDK-8201562
I understand it's not as efficient to inject a class in a package
different than the package of the transformed class. With the
principle of least privilege, I prefer not to provide an API to inject
any class in any package and you can achieve by calling
retransformClasses.
What do you think?
Mandy
On 4/18/18 5:23 AM, Rafael Winterhalter wrote:
Hei Mandy,
Lookup::defineClass would always be an alternative but it would
require me to open the class first. If the instrumented type can read
the module with the callback but its module was not opened, this
would not help me much, unfortunately. Also, I could not resolve this
lookup as the class in question is not necessarily loaded at this point.
Best regards, Rafael
2018-04-17 9:28 GMT+02:00 mandy chung <[email protected]
<mailto:[email protected]>>:
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 <[email protected]
<mailto:[email protected]>>:
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>
