8269770: nsk tests should start IOPipe channel before launch debuggee - Debugee.prepareDebugee #4659

alexmenkov · 2021-07-01T23:21:03Z

The change fixes several hundreds tests which launch debugee by using uses Debugee.prepareDebugee() method or use
debugee = Binder.bindToDebugee(...);
IOPipe pipe = debugee.createIOPipe();
logic.
Debugee.prepareDebugee() and Binder.bindToDebugee() launch debuggee by using CommandLineLaunch JDI connector with suspend=="true" argument, so they return debuggee suspended before the main class is loaded.
The fix starts IOPipe listening before debuggee VM is resumed.

Simplified isPackagePrivate/accipp001.java test to use Debugee.prepareDebugee() - it does exactly the same as Debugee.prepareDebugee() does (the only difference is using deprecated IOPipe ctor instead of Debugee.createIOPipe())

Tested all affected tests:
test/hotspot/jtreg/vmTestbase/nsk/jdi
test/hotspot/jtreg/vmTestbase/nsk/jdwp
test/hotspot/jtreg/serviceability/dcmd

Progress

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

Issue

JDK-8269770: nsk tests should start IOPipe channel before launch debuggee - Debugee.prepareDebugee

Reviewers

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

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 4659

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

Using diff file

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

bridgekeeper · 2021-07-01T23:21:52Z

👋 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 · 2021-07-01T23:24:33Z

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

mlbridge · 2021-07-01T23:28:15Z

Webrevs

01: Full - Incremental (854df1c9)
00: Full (8c46ee02)

sspitsyn · 2021-07-02T06:18:39Z

test/hotspot/jtreg/vmTestbase/nsk/share/jpda/DebugeeProcess.java

@@ -108,12 +108,16 @@ public Process getProcess() {

    // --------------------------------------------------- //

-    /** Created and return new IOPipe channel to the debugee VM. */
+    /** Created and return new IOPipe channel to the debuggee VM.


Not your typo but would be nice to fix this in the comment: "Created" => "Create"

openjdk · 2021-07-02T06:19:50Z

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

8269770: nsk tests should start IOPipe channel before launch debuggee - Debugee.prepareDebugee

Reviewed-by: sspitsyn, kevinw

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

d1aeca1: 8272541: Incorrect overflow test in Toom-Cook branch of BigInteger multiplication
dfeb413: 8272964: java/nio/file/Files/InterruptCopy.java fails with java.lang.RuntimeException: Copy was not interrupted
a033aa5: 8273072: Avoid using += in configure
b92214a: 8272480: Remove Mutex::access rank
596b075: 8258465: Headless build fails due to missing X11 headers on linux
a49a0c5: 8273062: Generation::refs_discovery_is_xxx functions are unused
ebd62bc: 8272846: Move some runtime/Metaspace/elastic/ tests out of tier1
46684a4: 8262386: resourcehogs/serviceability/sa/TestHeapDumpForLargeArray.java timed out
c925c7f: 8273020: LibraryCallKit::sharpen_unsafe_type does not handle narrow oop array
76baace: 8273045: Fix misc javadoc bugs in the java.security and javax.net.ssl code
... and 664 more: https://git.openjdk.java.net/jdk/compare/83bce94cc8a7fb45b0604598411fbecc62000dfd...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.

alexmenkov · 2021-07-26T20:17:21Z

Ping. Need 2nd reviewer

bridgekeeper · 2021-08-23T21:01:03Z

@alexmenkov This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

alexmenkov · 2021-08-26T22:43:31Z

Ping. Still looking for 2nd reviewer

kevinjwalls · 2021-08-27T17:21:39Z

OK... New to me but I think I follow enough to sanity check this.

I found that e.g. vmTestbase/nsk/share/jdi/Debugee.java defines prepareDebuggee() which calls debugee.createIOPipe(), which is changed here.
Previously, DebugeeProcess.java uses an IOPipe constructor which is marked deprecated.
Now calls IOPipe.startDebuggerPipe(Binder binder), which calls startListening().
In accipp001.java the prepareDebugee() call in runThis() handles everything it used to do itself.

alexmenkov · 2021-08-31T22:42:02Z

/integrate

openjdk · 2021-08-31T22:43:20Z

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

9c392d0: 8273197: ProblemList 2 jtools tests due to JDK-8273187
3d657eb: 8262186: Call X509KeyManager.chooseClientAlias once for all key types
c1e0aac: 8273186: Remove leftover comment about sparse remembered set in G1 HeapRegionRemSet
683e30d: 8273169: java/util/regex/NegativeArraySize.java failed after JDK-8271302
1996f64: 8273092: Sort classlist in JDK image
ba3587e: 8273144: Remove unused top level "Sample Collection Set Candidates" logging
75d987a: 8262095: NPE in Flow$FlowAnalyzer.visitApply: Cannot invoke getThrownTypes because tree.meth.type is null
e551852: 8271225: Add floorDivExact() method to java.lang.[Strict]Math
e671255: 8237567: Refactor G1-specific code in shared VM_CollectForMetadataAllocation
9bc7cc5: 8273033: SerialGC: remove obsolete comments
... and 697 more: https://git.openjdk.java.net/jdk/compare/83bce94cc8a7fb45b0604598411fbecc62000dfd...master

Your commit was automatically rebased without conflicts.

openjdk · 2021-08-31T22:43:31Z

@alexmenkov Pushed as commit 18a731a.

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

DebugeeProcess.createIOPipe starts listening

8c46ee0

openjdk bot added the rfr Pull request is ready for review label Jul 1, 2021

openjdk bot added the serviceability serviceability-dev@openjdk.org label Jul 1, 2021

sspitsyn approved these changes Jul 2, 2021

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jul 2, 2021

Fixed comment

854df1c

sspitsyn approved these changes Jul 6, 2021

View reviewed changes

kevinjwalls approved these changes Aug 27, 2021

View reviewed changes

openjdk bot closed this Aug 31, 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 Aug 31, 2021

alexmenkov deleted the IOPipe_prepareDebuggee branch August 31, 2021 23:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8269770: nsk tests should start IOPipe channel before launch debuggee - Debugee.prepareDebugee #4659

8269770: nsk tests should start IOPipe channel before launch debuggee - Debugee.prepareDebugee #4659

alexmenkov commented Jul 1, 2021 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jul 1, 2021

openjdk bot commented Jul 1, 2021

mlbridge bot commented Jul 1, 2021 •

edited

Loading

sspitsyn Jul 2, 2021

openjdk bot commented Jul 2, 2021 •

edited

Loading

alexmenkov commented Jul 26, 2021

bridgekeeper bot commented Aug 23, 2021

alexmenkov commented Aug 26, 2021

kevinjwalls commented Aug 27, 2021

alexmenkov commented Aug 31, 2021

openjdk bot commented Aug 31, 2021

openjdk bot commented Aug 31, 2021

8269770: nsk tests should start IOPipe channel before launch debuggee - Debugee.prepareDebugee #4659

8269770: nsk tests should start IOPipe channel before launch debuggee - Debugee.prepareDebugee #4659

Conversation

alexmenkov commented Jul 1, 2021 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Jul 1, 2021

openjdk bot commented Jul 1, 2021

mlbridge bot commented Jul 1, 2021 • edited Loading

Webrevs

sspitsyn Jul 2, 2021

Choose a reason for hiding this comment

openjdk bot commented Jul 2, 2021 • edited Loading

alexmenkov commented Jul 26, 2021

bridgekeeper bot commented Aug 23, 2021

alexmenkov commented Aug 26, 2021

kevinjwalls commented Aug 27, 2021

alexmenkov commented Aug 31, 2021

openjdk bot commented Aug 31, 2021

openjdk bot commented Aug 31, 2021

alexmenkov commented Jul 1, 2021 •

edited by openjdk bot

Loading

mlbridge bot commented Jul 1, 2021 •

edited

Loading

openjdk bot commented Jul 2, 2021 •

edited

Loading