On Wed, 3 Jul 2024 19:44:36 GMT, Chen Liang <li...@openjdk.org> wrote:

>> Since Java 5 the `java.lang.instrument` package provides services that allow 
>> Java programming language agents to instrument (i.e. modify the bytecode) of 
>> programs running on the Java Virtual Machine. The `java.lang.instrument` 
>> functionality is based and implemented on top of the native Java Virtual 
>> Machine Tool Interface (JVMTI) also introduced in Java 5. But because the 
>> `java.lang.instrument` API is a pure Java API and uses Java classes to 
>> instrument Java classes it imposes some usage restrictions which are not 
>> very well documented in its API specification.
>> 
>> E.g. the section on ["Bytecode 
>> Instrumentation"](https://docs.oracle.com/en/java/javase/21/docs/specs/jvmti.html#bci)
>>  in the JVMTI specification explicitly warns that special "*Care must be 
>> taken to avoid perturbing dependencies, especially when instrumenting core 
>> classes*". The risk of such "perturbing dependencies" is obviously much 
>> higher in a Java API like `java.lang.instrument`, but a more detailed 
>> explanation and warning is missing from its API documentation.
>> 
>> The most evident class file transformation restriction is that while a class 
>> A is being loaded and transformed it is not possible to use this same class 
>> directly or transitively from the `ClassFileTransformer::transform()` 
>> method. Violating this rule will result in a `ClassCircularityError` (the 
>> exact error type is disputable as can be seen in [8164165: JVM throws 
>> incorrect exception when ClassFileTransformer.transform() triggers class 
>> loading of class already being 
>> loaded](https://bugs.openjdk.org/browse/JDK-8164165), but the result would 
>> be a `LinkageError in any case).
>> 
>> The risk to run into such a `ClassCircularityError` error increases with the 
>> amount of code a transforming agent is transitively using from the 
>> `transform()` method. Using popular libraries like ASM, ByteBuddy, etc. for 
>> transformation further increases the probability of running into such 
>> issues, especially if the agent aims to transform core JDK library classes.
>> 
>> By default, the occurrence of a `ClassCircularityError` in 
>> `ClassFileTransformer::transform()` will be handled gracefully with the only 
>> consequence that the current transformation target will be loaded unmodified 
>> (see `ClassFileTransformer` API spec: "*throwing an exception has the same 
>> effect as returning null*"). But unfortunately, it can also have a subtle 
>> but at the same time much more far-reaching consequence. If the 
>> `ClassCircularityError` occurs during the resolution of a constant pool ...
>
> src/java.instrument/share/classes/java/lang/instrument/ClassFileTransformer.java
>  line 169:
> 
>> 167:  * Transforming core JDK classes or using libraries which depend on 
>> core JDK classes
>> 168:  * during transformation increases the risk for such errors. If the 
>> {@link LinkageError}
>> 169:  * occurs during reference resolution (see section 5.4.3 Resolution of 
>> <cite>The
> 
> Suggestion:
> 
>  * occurs during reference resolution (see section {@jvms 5.4.3} Resolution 
> of <cite>The
> 
> Other stylistic note: we can replace `<code>C</code>` with `{@code C}`, which 
> is shorter.

Thanks for your suggestion @liach . I didn't knew about the `@jvms` tag which 
is quite handy. I've also updated the other reference to the JVMS above the new 
`@apiNote` which didn't used the tag either (and was wrong because the JVMS 
sections have changed since Java 5 :)

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20011#discussion_r1665513865

Reply via email to