Skip to content
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

8280601: ClhsdbThreadContext.java test is triggering codecache related asserts #7217

Closed
wants to merge 2 commits into from

Conversation

plummercj
Copy link
Contributor

@plummercj plummercj commented Jan 25, 2022

It's possible for an address to be in the codecache but not in any CodeBlob. Don't assert in this case.

Also I ran into a couple of other asserts listed below. It looks like since dumping the threadcontext can result in using PointerFinder with fairly random addresses, it is doing a much better job of stress testing PointerFinder than is done by other tests. The root issue in all these asserts is that a random address in the codecache can either be outside of any CodeBlob, or can map to a CodeBlob that is not yet initialized or could even have been freed. PointerFinder and PointerLocation need to be prepared to handled these asserts, and any other exception thrown.

sun.jvm.hotspot.utilities.AssertionFailure: found wrong CodeBlob
at jdk.hotspot.agent/sun.jvm.hotspot.utilities.Assert.that(Assert.java:32)
at jdk.hotspot.agent/sun.jvm.hotspot.code.CodeCache.findBlobUnsafe(CodeCache.java:140)
at jdk.hotspot.agent/sun.jvm.hotspot.utilities.PointerFinder.find(PointerFinder.java:138)
at jdk.hotspot.agent/sun.jvm.hotspot.runtime.JavaThread.printThreadContextOn(JavaThread.java:493)
at jdk.hotspot.agent/sun.jvm.hotspot.CommandProcessor$46.doit(CommandProcessor.java:1702)
at jdk.hotspot.agent/sun.jvm.hotspot.CommandProcessor.executeCommand(CommandProcessor.java:2215)
at jdk.hotspot.agent/sun.jvm.hotspot.CommandProcessor.executeCommand(CommandProcessor.java:2185)
at jdk.hotspot.agent/sun.jvm.hotspot.CommandProcessor.run(CommandProcessor.java:2056)
at jdk.hotspot.agent/sun.jvm.hotspot.CLHSDB.run(CLHSDB.java:112)
at jdk.hotspot.agent/sun.jvm.hotspot.CLHSDB.main(CLHSDB.java:44)
at jdk.hotspot.agent/sun.jvm.hotspot.SALauncher.runCLHSDB(SALauncher.java:281)
at jdk.hotspot.agent/sun.jvm.hotspot.SALauncher.main(SALauncher.java:500)

sun.jvm.hotspot.utilities.AssertionFailure: Should have found CodeBlob
at jdk.hotspot.agent/sun.jvm.hotspot.utilities.Assert.that(Assert.java:32)
at jdk.hotspot.agent/sun.jvm.hotspot.utilities.PointerFinder.find(PointerFinder.java:140)
at jdk.hotspot.agent/sun.jvm.hotspot.runtime.JavaThread.printThreadContextOn(JavaThread.java:493)
at jdk.hotspot.agent/sun.jvm.hotspot.CommandProcessor$46.doit(CommandProcessor.java:1702)
at jdk.hotspot.agent/sun.jvm.hotspot.CommandProcessor.executeCommand(CommandProcessor.java:2215)
at jdk.hotspot.agent/sun.jvm.hotspot.CommandProcessor.executeCommand(CommandProcessor.java:2185)
at jdk.hotspot.agent/sun.jvm.hotspot.CommandProcessor.run(CommandProcessor.java:2056)
at jdk.hotspot.agent/sun.jvm.hotspot.CLHSDB.run(CLHSDB.java:112)
at jdk.hotspot.agent/sun.jvm.hotspot.CLHSDB.main(CLHSDB.java:44)
at jdk.hotspot.agent/sun.jvm.hotspot.SALauncher.runCLHSDB(SALauncher.java:281)
at jdk.hotspot.agent/sun.jvm.hotspot.SALauncher.main(SALauncher.java:500)


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • JDK-8280601: ClhsdbThreadContext.java test is triggering codecache related asserts

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/7217/head:pull/7217
$ git checkout pull/7217

Update a local copy of the PR:
$ git checkout pull/7217
$ git pull https://git.openjdk.java.net/jdk pull/7217/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 7217

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/7217.diff

@bridgekeeper
Copy link

bridgekeeper bot commented Jan 25, 2022

👋 Welcome back cjplummer! 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
Copy link

openjdk bot commented Jan 25, 2022

@plummercj The following label will be automatically applied to this pull request:

  • serviceability

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added the serviceability label Jan 25, 2022
@plummercj plummercj marked this pull request as ready for review Jan 26, 2022
@openjdk openjdk bot added the rfr label Jan 26, 2022
@mlbridge
Copy link

mlbridge bot commented Jan 26, 2022

Webrevs

Copy link
Contributor

@sspitsyn sspitsyn left a comment

Looks okay to me.
Thanks,
Serguei

@openjdk
Copy link

openjdk bot commented Jan 26, 2022

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

8280601: ClhsdbThreadContext.java test is triggering codecache related asserts

Reviewed-by: kevinw, sspitsyn

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

  • 9ca7ff3: 8281082: Improve javadoc references to JOSS
  • c74b8f4: 8275914: SHA3: changing java implementation to help C2 create high-performance code
  • a18beb4: 8280867: Cpuid1Ecx feature parsing is incorrect for AMD CPUs
  • fdd9ca7: 8280642: ObjectInputStream.readObject should throw InvalidClassException instead of IllegalAccessError
  • d95de5c: 8255495: Support CDS Archived Heap for uncompressed oops
  • bde2b37: 8279954: java/lang/StringBuffer(StringBuilder)/HugeCapacity.java intermittently fails
  • d1cc5fd: 8280941: os::print_memory_mappings() prints segment preceeding the inclusion range
  • 4532c3a: 8280554: resourcehogs/serviceability/sa/ClhsdbRegionDetailsScanOopsForG1.java can fail if GC is triggered
  • 5080e81: 8280770: serviceability/sa/ClhsdbThreadContext.java sometimes fails with 'Thread "SteadyStateThread"' missing from stdout/stderr
  • 1f6fcbe: 8278475: G1 dirty card refinement by Java threads may get unnecessarily paused
  • ... and 204 more: https://git.openjdk.java.net/jdk/compare/46fd683820bb7149c0605a0ba03f59e76de69c16...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.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot added the ready label Jan 26, 2022
@openjdk openjdk bot removed the ready label Jan 28, 2022
@plummercj
Copy link
Contributor Author

plummercj commented Jan 28, 2022

I ran across a couple more asserts due to what are essentially invalid pointers into the code cache. They look like allocated CodeBlobs, but could be partially initialized, or I think even deleted (or in the process of being deleted). PointerFinder and PointerLocation need to deal with asserts and exceptions that can result from this. The latest changes cover that. Changes are significant enough that I need to have everyone re-review. Thanks.

@kevinjwalls
Copy link

kevinjwalls commented Jan 28, 2022

Yes, looks good -- I like less asserting or throwing from methods that are trying to make sense of data that may not make perfect sense!

@plummercj
Copy link
Contributor Author

plummercj commented Feb 1, 2022

@sspitsyn Can re-review these changes? I made a few changes since your last review. Thanks.

@plummercj
Copy link
Contributor Author

plummercj commented Feb 1, 2022

Thanks Serguei and Kevin!

@plummercj
Copy link
Contributor Author

plummercj commented Feb 1, 2022

/integrate

@openjdk
Copy link

openjdk bot commented Feb 1, 2022

@plummercj This pull request has not yet been marked as ready for integration.

@plummercj plummercj changed the title 8280601: ClhsdbThreadContext.java test is triggering codecache related assert in PointerFinder.find() 8280601: ClhsdbThreadContext.java test is triggering codecache related asserts Feb 1, 2022
@openjdk openjdk bot added the ready label Feb 1, 2022
@plummercj
Copy link
Contributor Author

plummercj commented Feb 1, 2022

/integrate

@openjdk
Copy link

openjdk bot commented Feb 1, 2022

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

  • 9ca7ff3: 8281082: Improve javadoc references to JOSS
  • c74b8f4: 8275914: SHA3: changing java implementation to help C2 create high-performance code
  • a18beb4: 8280867: Cpuid1Ecx feature parsing is incorrect for AMD CPUs
  • fdd9ca7: 8280642: ObjectInputStream.readObject should throw InvalidClassException instead of IllegalAccessError
  • d95de5c: 8255495: Support CDS Archived Heap for uncompressed oops
  • bde2b37: 8279954: java/lang/StringBuffer(StringBuilder)/HugeCapacity.java intermittently fails
  • d1cc5fd: 8280941: os::print_memory_mappings() prints segment preceeding the inclusion range
  • 4532c3a: 8280554: resourcehogs/serviceability/sa/ClhsdbRegionDetailsScanOopsForG1.java can fail if GC is triggered
  • 5080e81: 8280770: serviceability/sa/ClhsdbThreadContext.java sometimes fails with 'Thread "SteadyStateThread"' missing from stdout/stderr
  • 1f6fcbe: 8278475: G1 dirty card refinement by Java threads may get unnecessarily paused
  • ... and 204 more: https://git.openjdk.java.net/jdk/compare/46fd683820bb7149c0605a0ba03f59e76de69c16...master

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated label Feb 1, 2022
@openjdk openjdk bot closed this Feb 1, 2022
@openjdk openjdk bot removed ready rfr labels Feb 1, 2022
@openjdk
Copy link

openjdk bot commented Feb 1, 2022

@plummercj Pushed as commit 85d839f.

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

@plummercj plummercj deleted the 8280601-sa_codecache branch Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrated serviceability
3 participants