-
Notifications
You must be signed in to change notification settings - Fork 147
8335619: Add an @apiNote to j.l.i.ClassFileTransformer to warn about recursive class loading and ClassCircularityErrors #2012
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
👋 Welcome back fandreuz! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
This backport pull request has now been updated with issue from the original commit. |
|
|
|
/approval request Fixes JDK-8335619. Adds useful information about potential issues for users of java.lang.instrument. Almost clean except for a conflict in the license header. |
|
Hi @fandreuz, |
|
HI @GoeLin, I don't think this backport requires a CSR. I've evaluated the need for a CSR in the initial PR and came to the conclusion (together with the reviewers) that it is not needed because:
Also, the |
|
@fandreuz This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a |
|
Although this issue manifests primarily when using Project Loom (Virtual Threads), that makes it even more critical. Loom is the flagship feature of JDK 21 and the main reason many enterprises are adopting this LTS release. With this bug, major instrumentation tools (AppDynamics, DataDog, New Relic, etc.) cannot function correctly on JDK 21 with Loom, effectively blocking safe adoption of one of the most important features in modern Java. I strongly urge the maintainers to prioritize the integration of this backport into 21u, as it is a blocker for enterprise production environments that depend on monitoring and observability. |
@Cirocc, I don't understand your comment. While I agree that this change should be downported, I don't understand how it can "unblock" monitoring/instrumentation tools because this PR only adds an API note to the documentation. |
|
@fandreuz This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a |
|
@fandreuz This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the |
Almost clean backport of JDK-8335619, except for a conflict in the copyright header (year). Adds useful information for users of
java.lang.instrument.Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk21u-dev.git pull/2012/head:pull/2012$ git checkout pull/2012Update a local copy of the PR:
$ git checkout pull/2012$ git pull https://git.openjdk.org/jdk21u-dev.git pull/2012/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 2012View PR using the GUI difftool:
$ git pr show -t 2012Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk21u-dev/pull/2012.diff
Using Webrev
Link to Webrev Comment