8307885: com/sun/jdi/ConnectedVMs.java fails with "Invalid debuggee exitValue: 0"

[plummercj]

When using -Xcomp, the 5 second sleep in the debuggee main thread is not long enough to guarantee using SIGTERM on the debuggee will make it exit(143) before the main thread exits cleanly with exitValue 0. The easiest solution is to just allow exitValue 0. See the CR for more details on other solutions I rejected.

---

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-8307885: com/sun/jdi/ConnectedVMs.java fails with "Invalid debuggee exitValue: 0"

Reviewers

• Kevin Walls (@kevinjwalls - Committer)
• Serguei Spitsyn (@sspitsyn - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 13923

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

Using diff file

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

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 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]

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

[plummercj]

/issue add JDK-8307885

[mlbridge]

Webrevs

• 00: Full (9334847d)

[openjdk]

@plummercj This issue is referenced in the PR title - it will now be updated.

[kevinjwalls]

Reasonable compromise 8-)

passName doesn't get passed to InstTarg, otherwise it could wait at the end in the "Kill" case, as if we want to see a process exit in response to a SIGTERM, then that process needs to wait around. But tolerating the signal not terminating it due to the race should be reliable also. 8-)

[openjdk]

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

8307885: com/sun/jdi/ConnectedVMs.java fails with "Invalid debuggee exitValue: 0"

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

• 2bf7ac5: 8297657: name demangling intermittently fails
• d20034b: 8307058: Implementation of Generational ZGC
• 0cbfbc4: 8307813: [JVMCI] Export markWord::lock_mask_in_place to JVMCI compilers.
• ba9714d: 8307610: Linker::nativeLinker should not be restricted (mainline)
• ecc1d85: 8307869: Remove unnecessary log statements from arm32 fastlocking code
• 08fa269: 8307349: Support xlc17 clang toolchain on AIX
• 8ac7186: 8305416: runtime/Thread/TestAlwaysPreTouchStacks.java failed with "Did not find expected NMT output"
• 984fbbb: 8307810: Consistently use LockingMode instead of UseHeavyMonitors
• 39f4e4d: 8307758: RISC-V: Improve bit test code introduced by JDK-8291555
• 3cb606e: 8306965: osThread allocation failures should not abort the VM
• ... and 3 more: https://git.openjdk.org/jdk/compare/cc9f7ad9ce33dc44d335fb7fb5483795c62ba936...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.

[plummercj]

passName doesn't get passed to InstTarg, otherwise it could wait at the end in the "Kill" case, as if we want to see a process exit in response to a SIGTERM, then that process needs to wait around

It would be easy to pass it to InstTarg.main(), but then you still have the issue of how long to sleep() and need to do it for 2 of the 4 tests cases, so it doesn't really help much.

[plummercj]

I'm going to push this a bit early since it is creating noise in our tier4 CI.

[plummercj]

Thanks for the reviews Kevin and Serguei!

/integrate

[openjdk]

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

• 9ad38cb: 8307626: java/net/httpclient/FlowAdapter* tests should close the HttpClient instances
• 2bf7ac5: 8297657: name demangling intermittently fails
• d20034b: 8307058: Implementation of Generational ZGC
• 0cbfbc4: 8307813: [JVMCI] Export markWord::lock_mask_in_place to JVMCI compilers.
• ba9714d: 8307610: Linker::nativeLinker should not be restricted (mainline)
• ecc1d85: 8307869: Remove unnecessary log statements from arm32 fastlocking code
• 08fa269: 8307349: Support xlc17 clang toolchain on AIX
• 8ac7186: 8305416: runtime/Thread/TestAlwaysPreTouchStacks.java failed with "Did not find expected NMT output"
• 984fbbb: 8307810: Consistently use LockingMode instead of UseHeavyMonitors
• 39f4e4d: 8307758: RISC-V: Improve bit test code introduced by JDK-8291555
• ... and 4 more: https://git.openjdk.org/jdk/compare/cc9f7ad9ce33dc44d335fb7fb5483795c62ba936...master

Your commit was automatically rebased without conflicts.

[openjdk]

@plummercj Pushed as commit 489658d.

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