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 <[email protected]>: > 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 <[email protected] <mailto:[email protected]>> >> 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 >> >>
