-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8188784: javax/management/notification/BroadcasterSupportDeadlockTest.java - TEST FAILED: deadlock #18687
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
8188784: javax/management/notification/BroadcasterSupportDeadlockTest.java - TEST FAILED: deadlock #18687
Conversation
….java - TEST FAILED: deadlock
|
👋 Welcome back kevinw! A progress list of the required criteria for merging this PR into |
|
@kevinjwalls 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: 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 no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
|
@kevinjwalls The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
Webrevs
|
plummercj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
test/jdk/javax/management/notification/BroadcasterSupportDeadlockTest.java
Show resolved
Hide resolved
|
Thanks for the reviews! |
|
/integrate |
|
Going to push as commit 58911cc. |
|
@kevinjwalls Pushed as commit 58911cc. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
This test incorrectly fails, although rarely, thinking its "thread 2" has deadlocked.
A change of sleep will likely fix this, but there are other issues, so cleaning up the test a little.
Remove the probe for the ManagementFactory class, to check we are on jdk5 or later. 8-)
When sleeping, sleep 100, not 1ms, we don't need to spin fast and actually race with the other thread.
We have a 1000 iteration loop, but don't seem to use it. We only check once then either return (pass), fail, or break (which is also fail). Use the loop to check for the status change, which is likely what was intended.
Show the stackframes on all failures.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/18687/head:pull/18687$ git checkout pull/18687Update a local copy of the PR:
$ git checkout pull/18687$ git pull https://git.openjdk.org/jdk.git pull/18687/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 18687View PR using the GUI difftool:
$ git pr show -t 18687Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/18687.diff
Webrev
Link to Webrev Comment