8268364: jmethod clearing should be done during unloading

[krk]

Clean backport of 8268364.

It fixes crashes when accessing jmethodIDs of a class being unloaded.

Here is the reproducer that crashes JVM in ~1 second without the patch, but works fine with it: gist.

This fix also resolves the issue reported at async-profiler/async-profiler#974 for Java 17.

Additional testing: release and fastdebug with JTREG_KEYWORDS="\!headful & \!external-dep & \!printer", no new test failures with the patch.

To run the repro:

javac Main.java
gcc -shared -I"$JAVA_HOME/include" -I"$JAVA_HOME/include/linux" -fPIC repro.cpp -orepro.so

java -agentpath:"$(pwd)/repro.so" -Xmx100m Main

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• JDK-8268364 needs maintainer approval

Warning

 ⚠️ Found leading lowercase letter in issue title for 8268364: jmethod clearing should be done during unloading

Issue

• JDK-8268364: jmethod clearing should be done during unloading (Bug - P4 - Approved)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk17u-dev.git pull/2839/head:pull/2839
$ git checkout pull/2839

Update a local copy of the PR:
$ git checkout pull/2839
$ git pull https://git.openjdk.org/jdk17u-dev.git pull/2839/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2839

View PR using the GUI difftool:
$ git pr show -t 2839

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk17u-dev/pull/2839.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

Hi @krk, welcome to this OpenJDK project and thanks for contributing!

We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing /signed in a comment in this pull request.

If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user krk" as summary for the issue.

If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing /covered in a comment in this pull request.

[openjdk]

@krk This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8268364: jmethod clearing should be done during unloading

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 41 new commits pushed to the master branch:

• 22f82b9: 8320675: PrinterJob/SecurityDialogTest.java hangs
• a5d9f49: 8296972: [macos13] java/awt/Frame/MaximizedToIconified/MaximizedToIconified.java: getExtendedState() != 6 as expected.
• 7161e29: 8332825: ubsan: guardedMemory.cpp:35:11: runtime error: null pointer passed as argument 2, which is declared to never be null
• 81b2c8d: 8286160: (fs) Files.exists returns unexpected results with C:\pagefile.sys because it's not readable
• 6f99109: 8331405: Shenandoah: Optimize ShenandoahLock with TTAS
• 3248a61: 8339548: GHA: RISC-V: Use Debian snapshot archive for bootstrap
• 51395cf: 8326611: Clean up vmTestbase/nsk/stress/stack tests
• 81ad499: 8294726: Update URLs in minefield tests
• 98586bb: 8331626: unsafe.cpp:162:38: runtime error in index_oop_from_field_offset_long - applying non-zero offset 4563897424 to null pointer
• d92d633: 8332720: ubsan: instanceKlass.cpp:3550:76: runtime error: member call on null pointer of type 'struct Array'
• ... and 31 more: https://git.openjdk.org/jdk17u-dev/compare/6f382d1fcfb514ab11e991ceb3c94967e8cbc054...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[openjdk]

This backport pull request has now been updated with issue from the original commit.

[krk]

/covered

[bridgekeeper]

Thank you! Please allow for a few business days to verify that your employer has signed the OCA. Also, please note that pull requests that are pending an OCA check will not usually be evaluated, so your patience is appreciated!

[openjdk]

⚠️ @krk This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.

[mlbridge]

Webrevs

• 00: Full (051316fd)

[openjdk]

@krk usage: /approval [<id>] (request|cancel) [<text>]

[krk]

/approval request

[openjdk]

@krk
8268364: The approval request has been created successfully.

[GoeLin]

Hi @krk,
thanks for contributing.
Please provide a useful fix request comment. See also https://wiki.openjdk.org/display/JDKUpdates/How+to+contribute+or+backport+a+fix
Removed the fix-request tag in the meantime.
Please also check the GHA check failures. If they are unrealated to your PR, you can tell in a comment that you want to proceed anyways, else I assume you did not check them.

[krk]

Hi @GoeLin,

Thanks for your input.

I intend to backport this into 11 and 8 too.

GHA failure at serviceability/sa/ClhsdbFindPC#id2 can be seen failing without the patch, e.g. in #2843 (comment):
Error: java.lang.RuntimeException: Unable to deduce type of thread from address 0x000000012f808810 (expected type JavaThread, CompilerThread, MonitorDeflationThread, ServiceThread, JvmtiAgentThread or CodeCacheSweeperThread).

This is unrelated to the current PR.

[krk]

/approval request Backporting to fix crash when accessing jmethodIDs of a class being unloaded.

[openjdk]

@krk
8268364: The approval request has been updated successfully.

[openjdk]

@krk usage: /approval [<id>] (request|cancel) [<text>]

[openjdk]

@krk usage: /approval [<id>] (request|cancel) [<text>]

[krk]

/approval request The fix is required to allow JVM TI tools, especially profilers, run safely with class unloading enabled. Low risk: a small localized change that only affects liveness of jmethodIDs.

[openjdk]

@krk
8268364: The approval request has been updated successfully.

[krk]

/integrate

[openjdk]

@krk
Your change (at version 051316f) is now ready to be sponsored by a Committer.

[shipilev]

There are a few linked issues to this bug, are they recording regressions, or? Do they need to be addressed/backported to 17u too?

[krk]

• JDK-8339725 is tangential but independent - either PR does not need the other.
• JDK-8267879 already in JDK 17.
• JDK-8268088 is comment only.

and others do not have patches.

[shipilev]

All right, thanks.

/sponsor

[openjdk]

Going to push as commit 35c1ebc.
Since your change was applied there have been 41 commits pushed to the master branch:

• 22f82b9: 8320675: PrinterJob/SecurityDialogTest.java hangs
• a5d9f49: 8296972: [macos13] java/awt/Frame/MaximizedToIconified/MaximizedToIconified.java: getExtendedState() != 6 as expected.
• 7161e29: 8332825: ubsan: guardedMemory.cpp:35:11: runtime error: null pointer passed as argument 2, which is declared to never be null
• 81b2c8d: 8286160: (fs) Files.exists returns unexpected results with C:\pagefile.sys because it's not readable
• 6f99109: 8331405: Shenandoah: Optimize ShenandoahLock with TTAS
• 3248a61: 8339548: GHA: RISC-V: Use Debian snapshot archive for bootstrap
• 51395cf: 8326611: Clean up vmTestbase/nsk/stress/stack tests
• 81ad499: 8294726: Update URLs in minefield tests
• 98586bb: 8331626: unsafe.cpp:162:38: runtime error in index_oop_from_field_offset_long - applying non-zero offset 4563897424 to null pointer
• d92d633: 8332720: ubsan: instanceKlass.cpp:3550:76: runtime error: member call on null pointer of type 'struct Array'
• ... and 31 more: https://git.openjdk.org/jdk17u-dev/compare/6f382d1fcfb514ab11e991ceb3c94967e8cbc054...master

Your commit was automatically rebased without conflicts.

[openjdk]

@shipilev @krk Pushed as commit 35c1ebc.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.