Hi Rafael,
Thanks for the additional use cases of Unsafe::defineClass and looking
through many different agents to identify their migration path.
Summarize the requirements you collected (please correct/add):
1. need a means to define auxiliary classes in the same runtime package
of an instrumented class, e.g. to bridge non-public members
2. need a means to define agent classes in a module that provides
similar functionality as appendToBootstrapClassLoader and
appendToSystemClassLoaderSearch
3. define a class in a different runtime package as the instrumented
class. It's still unclear how common this is (inject a subclass
that depends on a package-private constructor)
The proposed API requires the agent to redefine the class in a
package that the agent intends to inject its class into (e.g.
redefine java.util.List in order to define java.util.AgentDispatcher).
The proposed API requires more work than the existing
Unsafe::defineClass that can define a class in any package but
still plausible, right?
We should explore other alternatives, if any.
Mandy
[1]
http://mail.openjdk.java.net/pipermail/serviceability-dev/2018-April/023535.html
On 5/29/18 2:17 PM, Rafael Winterhalter wrote:
Hei Mandy,
after further investigation I have found several other use cases of
sun.misc.Unsafe::defineClass for which the proposal for JDK-8200559
would not currently offer a solution. I will try to describe a generic
case of the problem I found:
Java agents sometimes need to communicate state from instrumented
classes to a dispatcher that comes with the agent. However, a Java agent
cannot generally expect that any agent classes are visible to an
application as a Java agent is loaded on the class path whereas
application classes might not be loaded by the system class loader or
any of its children. Therefore, instrumented classes cannot typically
callback into the agent. This is typically solved by adding classes to
the bootstrap class loader which is universally visible.
Such a callback typically looks like this:
public abstract class Dispatcher {
public volatile Dispatcher dispatcher;
public abstract void receiveMessage(Object msg);
public static void sendMessage(Object msg) {
Dispatcher d = dispatcher;
if (d != null) d.receiveMessage(msg);
}
}
The agent can now register a subclass of the dispatcher in the field and
receive messages from any instrumented code throughout an application
without considering class loaders.
Adding this call would be trivial by using the
Instrumentation::appendToBootSearchPath method. However, using this
method has two major downsides:
1. The method accepts a file which must be stored on the local file
system. This can be really tricky in some cases, it is typically easier
to just define the classes as byte arrays and
sun.misc.Unsafe::defineClass is a way to avoid this problem.
2. Appending to the search path is only possible in the unnamed module
since Java 9. It is no longer possible to define the above dispatcher to
be located in java.util for instance as the new standard class loader
does not consider the new jar for known modules anymore. Defining the
dispatcher in a well known package has two major advantages:
a) It is not necessary to alter the module graph to make sure that
all modules with instrumented classes read the boot loaders unnamed
module. This requires a lot of additional maintenance which impacts
performance and memory use. Also, bugs can trigger security risks if
edges are added excessively. With defining a class in java.util, this
can be avoided since the java.base module is impliiclty read by any module.
b) Some class loaders filter packages that are considered to be
loaded, especially OSGi and JBoss modules class loaders. Those class
loaders would for example never attempt to find classes in the com.acme
package on the bootstrap loader which is why it has become a popular
practice to add classes to java.* packages for such universal dispatch.
There are dozens of such restricting class loaders and working around
them is nearly impossible as there are so many.
With the current consideration, it would instead be required to "pseudo
redefine" a class such as java.util.List only to inject the dispatcher
into the desired package. (Alternatively one can also open the module of
java.base and use a method handles lookup but this is a bad idea if the
Java agent runs on the unnamed system class loader.) Note that all this
is possible using standard API. It is even easier to do with JVMTI where
class definition is trivial.
For all the reasons I have mentioned in previous mails and also for this
additional use case I hope that adding a method for defining a class can
be added to the Instrumentation interface, it would definitely allow for
the cleanest, fastest and least error-prone migration while not inviting
to the mentioned work-arounds which are also currently favored by most
teams working with Java agents that I know of where many of them just
open jdk.internal.misc to use the new Unsafe class what is really
unfortunate as this is a chance to avoid use of internal API
alltogether. This choice would also reduce additional time for Java 11
being supported by all major production monitoring tools that could just
add simple switches to their code to move from Unsafe to Instrumentation.
It would also be trivial to implement with an interface extension like:
interface Instrumentation {
default Class<?> defineClass(byte[] bytes, ClassLoader loader,
ProtectionDomain pd) { throw new
UnsupportedOperationException("defineClass"); }
}
Thanks for considering my suggestion and your feedback!
Best regards, Rafael
2018-04-15 8:23 GMT+02:00 mandy chung <mandy.ch...@oracle.com
<mailto:mandy.ch...@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/~mchung/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>
