8305931: jdk/jfr/jcmd/TestJcmdDumpPathToGCRoots.java failed with "Expected chains but found none"

[roberttoyonaga]

This is a backport of openjdk/jdk@65be5e0

This change to JFR test code is needed because TestJcmdDumpPathToGCRoots transiently fails because it can't guarantee the expected objects are sampled. The change improves cleanups between checks and also adds retries.

This should help resolve some test failures for Adoptium (see adoptium/aqa-tests#2766)

Testing: the updated test/jdk/jdk/jfr/jcmd/TestJcmdDumpPathToGCRoots.java test passes.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• JDK-8305931 needs maintainer approval
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8305931: jdk/jfr/jcmd/TestJcmdDumpPathToGCRoots.java failed with "Expected chains but found none" (Bug - P4 - Approved)

Reviewers

• Thomas Stuefe (@tstuefe - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 490

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

Using diff file

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

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back roberttoyonaga! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

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

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

8305931: jdk/jfr/jcmd/TestJcmdDumpPathToGCRoots.java failed with "Expected chains but found none"

Reviewed-by: stuefe

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 17 new commits pushed to the master branch:

• ddce2b5: 8299677: Formatter.format might take a long time to format an integer or floating-point
• c8eb027: 8075511: Enable -Woverloaded-virtual C++ warning for HotSpot build
• 16ea85b: 8335552: [8u] JDK-8303466 backport to 8u requires 3 ::Identity signature fixes
• 54f7734: 8303466: C2: failed: malformed control flow. Limit type made precise with MaxL/MinL
• 385e34d: 8262017: C2: assert(n != __null) failed: Bad immediate dominator info.
• 44eac48: 8137329: [windows] Build broken on VS2010 after "8046148: JEP 158: Unified JVM Logging"
• a0715ab: 8238274: (sctp) JDK-7118373 is not fixed for SctpChannel
• b499ea7: 8030204: com/sun/jdi/JdbExprTest.sh: Required output "Can\'t convert 2147483648 to int" not found
• b1e2ea8: 8279164: Disable TLS_ECDH_* cipher suites
• 6abb3f2: 8035395: sun/management/jmxremote/startstop/JMXStartStopTest.java fails intermittently: Port already in use
• ... and 7 more: https://git.openjdk.org/jdk8u-dev/compare/663ecc703dd8526178e3aab0036ddeac8767ff0f...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. Possible candidates are the reviewers of this PR (@tstuefe) but any other Committer may sponsor as well.

➡️ 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.

[mlbridge]

Webrevs

• 01: Full - Incremental (95c8f1f2)
• 00: Full (615f99db)

[bridgekeeper]

@roberttoyonaga 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 add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[roberttoyonaga]

Commenting so that this won't be closed due to inactivity. Could someone please help review this? I didn't have to make any additional changes, the backport was applied cleanly.

[tstuefe]

Hi Robert, what's wrong with the red GHA tests?

Besides that, please make sure, especially for a PR that has been dormant for a longer time, that you merge in the latest changes from upstream (we prefer merging over rebasing for a PR that is being discussed). Then the GHAs should be green, or the reds explainable. Then you can apply for approval (slash approval).

The fix itself looks good.

[roberttoyonaga]

Hi Thomas, I've pulled in the latest changes from master. With regard to the failing GHAs:

• Linux additional (hs s390x build only) / Linux additional (hs ppc64le build only)

  • Fails in "Create sysroot" step with: Cannot check Release signature; keyring file not available /usr/share/keyrings/debian-archive-keyring.gpg. It seems like this is some sort of environment setup / infrastructure problem.

• Windows x86 (build debug) / Windows x86 (build release)

  • Fails in Download and unpack Visual Studio 2010 step with: Extracting archive: C:\Users\runneradmin\VS2010Express1.iso Can't open as archive: 1. It looks like the Visual Studio download source is no longer good.

• macOS x64 (jdk/tier1) / Linux x64 (jdk/tier1) / Linux x86 (jdk/tier1) / Windows x64 (jdk/tier1)

  • Fails security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#teliasonerarootcav1 with: Test threw exception: java.lang.RuntimeException: Intermediate Root CA not found in the chain
  • I'm able to reproduce this locally on a clean master branch, so the failure seems independent of this PR. It looks like the URL used in the test (https://juolukka.cover.sonera.net:10443) needs to be updated, but I'm not sure what to change it to.

• Windows x64 (hotspot/tier1)

  • Failsruntime/CompressedOops/CompressedClassPointers.java with:
    Execution failed: main threw exception: java.lang.RuntimeException: 'Narrow klass base: 0x0000000000000000, Narrow klass shift: 3' missing from stdout/stderr
  • This doesn't seem related to the changes here since this PR only modifies JFR test code and doesn't affect compressed class pointers.

• Linux x86 (hotspot/tier1)

  • Fails gc/concurrentMarkSweep/SystemGCOnForegroundCollector.java with: Error. Program /home/runner/jdk-linux-x86/jdk-1.8.0-internal+0_linux-x86_bin/j2sdk-image/bin/java' timed out
  • This also doesn't seem related to the changes here since this PR does not touch any GC-related code.

[tstuefe]

@roberttoyonaga Okay, thank you for checking!

LGTM

[openjdk]

⚠️ @roberttoyonaga 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.

[roberttoyonaga]

/approval request for the backport of a JFR test fix for TestJcmdDumpPathToGCRoots. This only affects test code so should be a low risk. The patch is clean except for some file path changes.

[openjdk]

@roberttoyonaga
8305931: The approval request has been created successfully.

[bridgekeeper]

@roberttoyonaga 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 add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[roberttoyonaga]

Commenting to prevent closure due to inactivity.
JDK-8305931 still needs maintainer approval.

[jerboaa]

/approve yes

[openjdk]

@jerboaa
8305931: The approval request has been approved.

[roberttoyonaga]

/integrate

[openjdk]

@roberttoyonaga
Your change (at version 95c8f1f) is now ready to be sponsored by a Committer.

[jerboaa]

/sponsor

[openjdk]

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

• ddce2b5: 8299677: Formatter.format might take a long time to format an integer or floating-point
• c8eb027: 8075511: Enable -Woverloaded-virtual C++ warning for HotSpot build
• 16ea85b: 8335552: [8u] JDK-8303466 backport to 8u requires 3 ::Identity signature fixes
• 54f7734: 8303466: C2: failed: malformed control flow. Limit type made precise with MaxL/MinL
• 385e34d: 8262017: C2: assert(n != __null) failed: Bad immediate dominator info.
• 44eac48: 8137329: [windows] Build broken on VS2010 after "8046148: JEP 158: Unified JVM Logging"
• a0715ab: 8238274: (sctp) JDK-7118373 is not fixed for SctpChannel
• b499ea7: 8030204: com/sun/jdi/JdbExprTest.sh: Required output "Can\'t convert 2147483648 to int" not found
• b1e2ea8: 8279164: Disable TLS_ECDH_* cipher suites
• 6abb3f2: 8035395: sun/management/jmxremote/startstop/JMXStartStopTest.java fails intermittently: Port already in use
• ... and 7 more: https://git.openjdk.org/jdk8u-dev/compare/663ecc703dd8526178e3aab0036ddeac8767ff0f...master

Your commit was automatically rebased without conflicts.

[openjdk]

@jerboaa @roberttoyonaga Pushed as commit fc15e99.

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