On Tue, 9 Jul 2024 12:17:25 GMT, Alan Bateman <al...@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
