jdk8u-dev icon indicating copy to clipboard operation
jdk8u-dev copied to clipboard

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

Open roberttoyonaga opened this issue 1 year ago • 4 comments

This is a backport of https://github.com/openjdk/jdk/commit/65be5e0c547d74ca7de288b164aa9bd6d6855685

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 https://github.com/adoptium/aqa-tests/issues/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
  • [x] Change must not contain extraneous whitespace
  • [x] Commit message must refer to an issue

Issue

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

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

roberttoyonaga avatar May 03 '24 20:05 roberttoyonaga

:wave: 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.

bridgekeeper[bot] avatar May 03 '24 20:05 bridgekeeper[bot]

@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:

  • ddce2b5d39cb780c84d0c4d3bf01df2fe5d3d989: 8299677: Formatter.format might take a long time to format an integer or floating-point
  • c8eb027b59dbb12150a1edd926b14fc2f6753bbd: 8075511: Enable -Woverloaded-virtual C++ warning for HotSpot build
  • 16ea85b54115073c6c22377b817878fe112b2100: 8335552: [8u] JDK-8303466 backport to 8u requires 3 ::Identity signature fixes
  • 54f7734c7cea3a20d49103c4387ed9023992e5a6: 8303466: C2: failed: malformed control flow. Limit type made precise with MaxL/MinL
  • 385e34d860965a77df355edfd8c1d99af5afe38c: 8262017: C2: assert(n != __null) failed: Bad immediate dominator info.
  • 44eac48705dd89f20cda5c4384cbecf417555eb1: 8137329: [windows] Build broken on VS2010 after "8046148: JEP 158: Unified JVM Logging"
  • a0715ab4bc71642203324a2c06774aacee35fda7: 8238274: (sctp) JDK-7118373 is not fixed for SctpChannel
  • b499ea7ec2ffb2211a67d38b3bd689a77866cb0d: 8030204: com/sun/jdi/JdbExprTest.sh: Required output "Can\'t convert 2147483648 to int" not found
  • b1e2ea81f45d9b144e1de8d16b2b698f1788d4d7: 8279164: Disable TLS_ECDH_* cipher suites
  • 6abb3f27b9a535f741c7d3a8dfa0d6b811263a9e: 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[bot] avatar May 03 '24 20:05 openjdk[bot]

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

openjdk[bot] avatar May 03 '24 20:05 openjdk[bot]

Webrevs

mlbridge[bot] avatar May 03 '24 20:05 mlbridge[bot]

@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!

bridgekeeper[bot] avatar Jun 01 '24 01:06 bridgekeeper[bot]

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.

roberttoyonaga avatar Jun 04 '24 13:06 roberttoyonaga

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.

tstuefe avatar Jun 04 '24 16:06 tstuefe

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.

roberttoyonaga avatar Jun 04 '24 21:06 roberttoyonaga

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

openjdk[bot] avatar Jun 05 '24 15:06 openjdk[bot]

/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.

roberttoyonaga avatar Jun 05 '24 16:06 roberttoyonaga

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

openjdk[bot] avatar Jun 05 '24 16:06 openjdk[bot]

@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!

bridgekeeper[bot] avatar Jul 03 '24 16:07 bridgekeeper[bot]

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

roberttoyonaga avatar Jul 03 '24 17:07 roberttoyonaga

/approve yes

jerboaa avatar Jul 09 '24 15:07 jerboaa

@jerboaa 8305931: The approval request has been approved.

openjdk[bot] avatar Jul 09 '24 15:07 openjdk[bot]

/integrate

roberttoyonaga avatar Jul 09 '24 15:07 roberttoyonaga

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

openjdk[bot] avatar Jul 09 '24 15:07 openjdk[bot]

/sponsor

jerboaa avatar Jul 09 '24 15:07 jerboaa

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

  • ddce2b5d39cb780c84d0c4d3bf01df2fe5d3d989: 8299677: Formatter.format might take a long time to format an integer or floating-point
  • c8eb027b59dbb12150a1edd926b14fc2f6753bbd: 8075511: Enable -Woverloaded-virtual C++ warning for HotSpot build
  • 16ea85b54115073c6c22377b817878fe112b2100: 8335552: [8u] JDK-8303466 backport to 8u requires 3 ::Identity signature fixes
  • 54f7734c7cea3a20d49103c4387ed9023992e5a6: 8303466: C2: failed: malformed control flow. Limit type made precise with MaxL/MinL
  • 385e34d860965a77df355edfd8c1d99af5afe38c: 8262017: C2: assert(n != __null) failed: Bad immediate dominator info.
  • 44eac48705dd89f20cda5c4384cbecf417555eb1: 8137329: [windows] Build broken on VS2010 after "8046148: JEP 158: Unified JVM Logging"
  • a0715ab4bc71642203324a2c06774aacee35fda7: 8238274: (sctp) JDK-7118373 is not fixed for SctpChannel
  • b499ea7ec2ffb2211a67d38b3bd689a77866cb0d: 8030204: com/sun/jdi/JdbExprTest.sh: Required output "Can\'t convert 2147483648 to int" not found
  • b1e2ea81f45d9b144e1de8d16b2b698f1788d4d7: 8279164: Disable TLS_ECDH_* cipher suites
  • 6abb3f27b9a535f741c7d3a8dfa0d6b811263a9e: 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[bot] avatar Jul 09 '24 15:07 openjdk[bot]

@jerboaa @roberttoyonaga Pushed as commit fc15e99a3218a88434c27da1d7d09a01c592d8f9.

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

openjdk[bot] avatar Jul 09 '24 15:07 openjdk[bot]