8262454: Handshake timeout improvements, single target, kill unfinished thread #2743

robehn · 2021-02-26T10:48:14Z

When executing a single target handshake we do not have a timeout.
When we timeout we should crash the thread with the non-completed handshake operation.

VM_Handshake only had one implementer so I remove it.

Passes t1-5.

Progress

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

Issue

JDK-8262454: Handshake timeout improvements, single target, kill unfinished thread

Reviewers

Patricio Chilano Mateo (@pchilano - Committer) ⚠️ Review applies to c218c22
David Holmes (@dholmes-ora - Reviewer)
Daniel D. Daugherty (@dcubed-ojdk - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/2743/head:pull/2743
$ git checkout pull/2743

bridgekeeper · 2021-02-26T10:48:48Z

👋 Welcome back rehn! 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 · 2021-02-26T10:50:09Z

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

hotspot-runtime

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.

mlbridge · 2021-03-08T15:58:17Z

Webrevs

pchilano

Looks good!

test/hotspot/jtreg/runtime/handshake/HandshakeTimeoutTest.java

robehn · 2021-03-09T07:23:08Z

Looks good!

Thanks!

dholmes-ora

Overall this looks good. I'll approve the final version.

Thanks,
David

src/hotspot/share/runtime/handshake.cpp

test/hotspot/jtreg/runtime/handshake/HandshakeTimeoutTest.java

robehn · 2021-03-10T08:55:07Z

Overall this looks good. I'll approve the final version.

Thanks,
David

Thanks!

dholmes-ora

One typo below otherwise LGTM!

Thanks,
David

test/hotspot/jtreg/runtime/handshake/HandshakeTimeoutTest.java

openjdk · 2021-03-10T09:45:32Z

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

8262454: Handshake timeout improvements, single target, kill unfinished thread

Reviewed-by: pchilanomate, dholmes, dcubed

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

f3bd801: 8263403: [JVMCI] output written to tty via HotSpotJVMCIRuntime can be garbled
b92abac: 8263433: Shenandoah: Don't expect forwarded objects in set_concurrent_mark_in_progress()
15dacca: 8263465: JDK-8236847 causes tier1 build failure on linux-aarch64
7ed46bd: 8241716: Jpackage functionality to let users choose whether to create shortcuts
3820ab9: 8236847: CDS archive with 4K alignment unusable on machines with 64k pages
273f8bd: 8263248: IGV: accept graphs without node categories
a9b4f03: 8263069: Exclude some failing tests from security/infra/java/security/cert/CertPathValidator
470b150: 8143041: Unify G1CollectorPolicy::PauseKind and G1YCType
f6b4ba0: 8261931: IGV: quick search fails on multi-line node labels

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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.

robehn · 2021-03-10T13:18:12Z

One typo below otherwise LGTM!

Thanks,
David

Thanks!

dcubed-ojdk

I thought I had submitted this yesterday. Sorry about that.

src/hotspot/share/runtime/handshake.cpp

test/hotspot/jtreg/runtime/handshake/HandshakeTimeoutTest.java

robehn · 2021-03-11T08:10:36Z

I thought I had submitted this yesterday. Sorry about that.

Thanks!

dcubed-ojdk

Thumbs up. Minor nit about the sort order of the imports in the new test remains.

test/hotspot/jtreg/runtime/handshake/HandshakeTimeoutTest.java

dcubed-ojdk

Thumbs up.

robehn · 2021-03-12T08:57:56Z

/integrate

openjdk · 2021-03-12T08:58:46Z

@robehn Since your change was applied there have been 14 commits pushed to the master branch:

ff25939: 8263426: Reflow JfrNetworkUtilization::send_events
e25ad73: 8263430: Uninitialized Method* variables after JDK-8233913
9f6b1d7: 8263436: Silly array comparison in GaloisCounterMode.overlapDetection
ad1f605: 8263353: assert(CompilerOracle::option_matches_type(option, value)) failed: Value must match option type
cf1c021: 8263480: ProblemList two jpackage tests on Windows
f3bd801: 8263403: [JVMCI] output written to tty via HotSpotJVMCIRuntime can be garbled
b92abac: 8263433: Shenandoah: Don't expect forwarded objects in set_concurrent_mark_in_progress()
15dacca: 8263465: JDK-8236847 causes tier1 build failure on linux-aarch64
7ed46bd: 8241716: Jpackage functionality to let users choose whether to create shortcuts
3820ab9: 8236847: CDS archive with 4K alignment unusable on machines with 64k pages
... and 4 more: https://git.openjdk.java.net/jdk/compare/7988c1d9aa7e3b990c17fdf5e31195e66e7fc5f2...master

Your commit was automatically rebased without conflicts.

Pushed as commit aa33443.

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

openjdk bot added the hotspot-runtime hotspot-runtime-dev@openjdk.org label Feb 26, 2021

robehn force-pushed the 8262454-hs-timeout-st branch 2 times, most recently from 95ad267 to 534aeb3 Compare March 8, 2021 08:29

Fixed timeout

64fc6b8

robehn force-pushed the 8262454-hs-timeout-st branch from 534aeb3 to 64fc6b8 Compare March 8, 2021 14:00

robehn marked this pull request as ready for review March 8, 2021 15:55

openjdk bot added the rfr Pull request is ready for review label Mar 8, 2021

pchilano reviewed Mar 8, 2021

View reviewed changes

test/hotspot/jtreg/runtime/handshake/HandshakeTimeoutTest.java Outdated Show resolved Hide resolved

Merge branch 'master' into 8262454-hs-timeout-st

9b975a6

dholmes-ora reviewed Mar 10, 2021

View reviewed changes

src/hotspot/share/runtime/handshake.cpp Show resolved Hide resolved

test/hotspot/jtreg/runtime/handshake/HandshakeTimeoutTest.java Outdated Show resolved Hide resolved

robehn added 3 commits March 10, 2021 09:11

Use WB waitUnsafe to trigger timeout

f409d63

Merge branch 'master' into 8262454-hs-timeout-st

24ef011

Fixed cr year

2ab7ce8

dholmes-ora approved these changes Mar 10, 2021

View reviewed changes

test/hotspot/jtreg/runtime/handshake/HandshakeTimeoutTest.java Outdated Show resolved Hide resolved

openjdk bot added the ready Pull request is ready to be integrated label Mar 10, 2021

Spelling error

c218c22

pchilano approved these changes Mar 10, 2021

View reviewed changes

dcubed-ojdk approved these changes Mar 10, 2021

View reviewed changes

robehn added 2 commits March 11, 2021 08:47

Added comment, sorted imports in test

c3b151d

Merge branch 'master' into 8262454-hs-timeout-st

e6585f9

dholmes-ora approved these changes Mar 11, 2021

View reviewed changes

dcubed-ojdk approved these changes Mar 11, 2021

View reviewed changes

test/hotspot/jtreg/runtime/handshake/HandshakeTimeoutTest.java Outdated Show resolved Hide resolved

Sorted imports

1916520

dcubed-ojdk approved these changes Mar 11, 2021

View reviewed changes

dholmes-ora approved these changes Mar 11, 2021

View reviewed changes

openjdk bot closed this Mar 12, 2021

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Mar 12, 2021

robehn deleted the 8262454-hs-timeout-st branch March 17, 2021 07:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8262454: Handshake timeout improvements, single target, kill unfinished thread #2743

8262454: Handshake timeout improvements, single target, kill unfinished thread #2743

robehn commented Feb 26, 2021 •

edited by openjdk bot

Loading

bridgekeeper bot commented Feb 26, 2021

openjdk bot commented Feb 26, 2021

mlbridge bot commented Mar 8, 2021 •

edited

Loading

pchilano left a comment

robehn commented Mar 9, 2021

dholmes-ora left a comment

robehn commented Mar 10, 2021

dholmes-ora left a comment •

edited

Loading

openjdk bot commented Mar 10, 2021 •

edited

Loading

robehn commented Mar 10, 2021

dcubed-ojdk left a comment

robehn commented Mar 11, 2021

dcubed-ojdk left a comment

dcubed-ojdk left a comment

robehn commented Mar 12, 2021

openjdk bot commented Mar 12, 2021

8262454: Handshake timeout improvements, single target, kill unfinished thread #2743

8262454: Handshake timeout improvements, single target, kill unfinished thread #2743

Conversation

robehn commented Feb 26, 2021 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Download

bridgekeeper bot commented Feb 26, 2021

openjdk bot commented Feb 26, 2021

mlbridge bot commented Mar 8, 2021 • edited Loading

Webrevs

pchilano left a comment

Choose a reason for hiding this comment

robehn commented Mar 9, 2021

dholmes-ora left a comment

Choose a reason for hiding this comment

robehn commented Mar 10, 2021

dholmes-ora left a comment • edited Loading

Choose a reason for hiding this comment

openjdk bot commented Mar 10, 2021 • edited Loading

robehn commented Mar 10, 2021

dcubed-ojdk left a comment

Choose a reason for hiding this comment

robehn commented Mar 11, 2021

dcubed-ojdk left a comment

Choose a reason for hiding this comment

dcubed-ojdk left a comment

Choose a reason for hiding this comment

robehn commented Mar 12, 2021

openjdk bot commented Mar 12, 2021

robehn commented Feb 26, 2021 •

edited by openjdk bot

Loading

mlbridge bot commented Mar 8, 2021 •

edited

Loading

dholmes-ora left a comment •

edited

Loading

openjdk bot commented Mar 10, 2021 •

edited

Loading