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

[Volker Simonis]

On Tue, 9 Jul 2024 12:17:25 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> @AlanBateman would you mind having a quick look at this trivial, documentation-only PR and let me know if you're OK with it?
>> 
>> Thank you and best regards,
>> Volker
>
>> @AlanBateman would you mind having a quick look at this trivial, documentation-only PR and let me know if you're OK with it?
> 
> (Catching up as I was away for a few days). 
> 
> I agree it would be useful to have an API note on this topic. There were several reports of deadlocks, ClassCircularityError, and other hard to diagnose issues when support for instrumentation was originally introduced in JDK 5. I think agent maintainers learned to exclude many of the core classes in java.base but that is always a moving target and there have been a few more sightings/reports recently (maybe due to newer features doing more in Java rather than in the VM).
> 
> I agree that the ClassFileTransformer class description is the best place for this. My initial reaction was to wonder about the j.l.instrument package description but it is more focused on starting agents and the JAR file attributes so I think your proposed location is best.. 
> 
> As regards the text then it might be better to start with a sentence to say that the "transform" method may be called from critical points in JDK core classes or called to transform JDK core classes. You've got some of this in second sentence but I think better to lead with it before revealing one of the several possible things that can happen. I think part of the lead in needs to say "great care must be taken" or something like that to get across the point that the ClassFileTransformer implementor has the potential to cause many possible issues. For the same class and linkage error then probably best to not use the phrase "transitively requires" as it's too close "requires transitive" used in module declarations. 
> 
> Note that the `@jvms` tag can be used to reference the JVMS section. You'll see a few examples in other classes.

Thanks a lot for looking into this @AlanBateman. I've tried to integrate your suggestions into the new version of the PR.

I already used the `@jvms` tag in the latest version of my PR based on @liach's suggestion. I 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 :)

Please let me know if this sounds better now or if you see further room for improvement.

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

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