8361751: Test sun/tools/jcmd/TestJcmdSanity.java timed out on Windows

[alexmenkov]

Tests which run serviceability tools to attach to the main test process can get deadlock with streaming output (see #24672 )
The fix disables attach streaming output for tests in jdk/sun/tools/jcmd

Testing: tier6, tier7

---

Progress

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

Issue

• JDK-8361751: Test sun/tools/jcmd/TestJcmdSanity.java timed out on Windows (Bug - P3)

Reviewers

• Chris Plummer (@plummercj - Reviewer)
• David Holmes (@dholmes-ora - Reviewer)
• Serguei Spitsyn (@sspitsyn - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26358/head:pull/26358
$ git checkout pull/26358

Update a local copy of the PR:
$ git checkout pull/26358
$ git pull https://git.openjdk.org/jdk.git pull/26358/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26358

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26358.diff

Using Webrev

Link to Webrev Comment

[alexmenkov]

/label add serviceability

[bridgekeeper]

👋 Welcome back amenkov! 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]

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

8361751: Test sun/tools/jcmd/TestJcmdSanity.java timed out on Windows

Reviewed-by: cjplummer, dholmes, 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 51 new commits pushed to the master branch:

• 4e0b035: 8338282: javax/swing/JMenuBar/TestMenuMnemonicLinuxAndMac.java test failed on macOS and Ubuntu
• a23987f: 8361283: [Accessibility,macOS,VoiceOver] VoiceOver announced Tab items of JTabbedPane as RadioButton on macOS
• 04c0b13: 8362565: ProblemList jdk/jfr/event/io/TestIOTopFrame.java
• ... and 48 more: https://git.openjdk.org/jdk/compare/9bef2d1610647dec18f9e81cbac3dddbbf99dd6d...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]

@alexmenkov
The serviceability label was successfully added.

[mlbridge]

Webrevs

• 00: Full (7afa3676)

[dholmes-ora]

Okay, but are there other tests we have missed here?

I'm wondering what the value of "streaming output" is when it can lead to deadlocks? Is this just a test problem?

Thanks

[alexmenkov]

Okay, but are there other tests we have missed here?

I'd say this tests was missed in https://bugs.openjdk.org/browse/JDK-8354461 fix.

I'm wondering what the value of "streaming output" is when it can lead to deadlocks? Is this just a test problem?

"Streaming output" means attach operation output is not buffered, but is sent to the client while the handler produce it.
It's useful in real cases, when user runs jcmd or other tool against external target VM (see https://bugs.openjdk.org/browse/JDK-8319055 and related issues).
In tests we often use scenarios when test attaches to itself or like in this case use attach tool to attach to the test process.
And we can get deadlock because producer (operation handler, often executed in safepoint) and consumer (test reads redirected output of the tool) are executed in the same process. It's just test problem (to reproduce real scenario tests should run target VM as a separate process)

[alexmenkov]

/integrate

[openjdk]

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

• 60c29ff: 8362524: Fix confusing but harmless typos in x86 CPU Features
• 30d2003: 8357380: java/lang/StringBuilder/RacingSBThreads.java times out with C1
• 9dc6282: 8362169: Pointer passed to upcall may get wrong scope
• ... and 53 more: https://git.openjdk.org/jdk/compare/9bef2d1610647dec18f9e81cbac3dddbbf99dd6d...master

Your commit was automatically rebased without conflicts.

[openjdk]

@alexmenkov Pushed as commit a3843e8.

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