RFR: 8335619: Add an @apiNote to j.l.i.ClassFileTransformer to warn about recursive class loading and ClassCircularityErrors [v2]

[Volker Simonis]

On Thu, 4 Jul 2024 10:49:47 GMT, Volker Simonis <simonis at 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 ...
>
> Volker Simonis has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fixed text based on reviewer comments. Also added a '@jvms' tag to another reference to the JVMS and fixed its section number

Notice that according to the [CSR FAQ](https://wiki.openjdk.org/display/csr/CSR+FAQs), I don't think that this change requires a CSR because it is not changing the specification but merely describes the actual behavior in some more detail:

> Q: If the text of the javadoc of a public exported API is changing, is a CSR request needed?
A: *A CSR request is required if the specification of a public exported API. Not all javadoc updates are specification changes. For example, typo fixes and rephrasings that do not alter the semantics of the API in question do not require CSR review.*

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

PR Comment: https://git.openjdk.org/jdk/pull/20011#issuecomment-2208685235