On Thu, 26 Oct 2023 07:51:56 GMT, Johannes Bechberger <jbechber...@openjdk.org> 
wrote:

>> Fix race condition in debugger port selection, introduced with 
>> [JDK-8317920](https://bugs.openjdk.org/browse/JDK-8317920).
>> 
>> Tested on my Mac M1, but it doesn't contain platform-dependent code.
>
> Johannes Bechberger has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Add suggested fixes

test/jdk/com/sun/jdi/lib/jdb/Debuggee.java line 107:

> 105:         }
> 106: 
> 107:         // required to pass non null port with address and emit string 
> before the throw

The comment is obsolete

test/jdk/com/sun/jdi/lib/jdb/Debuggee.java line 129:

> 127: 
> 128:         public Debuggee launch(String name) {
> 129:             return new Debuggee(prepare(), name, s -> {

Address detector functions can be passed without wrapping:
`
            return new Debuggee(prepare(), name,
                                              onthrow.isEmpty() ? 
JDWP::parseListenAddress : Debuggee::parseLaunchEchoListenAddress)
`
But maybe current code is more readable, so it's up to you

test/jdk/com/sun/jdi/lib/jdb/Debuggee.java line 141:

> 139:     }
> 140: 
> 141:     // starts the process, waits for "Listening for transport" output 
> and detects transport/address

Suggestion:

    // starts the process, waits until the provided addressDetector detects 
transport/address from the process output

test/jdk/com/sun/jdi/lib/jdb/Debuggee.java line 211:

> 209:      * Returns null if the string specified does not contain required 
> info.
> 210:      */
> 211:     private static JDWP.ListenAddress 
> parseLaunchEchoListenAddress(String debuggeeOutput) {

The logic belongs to Launcher, please move the method to Launcher class

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16358#discussion_r1373538027
PR Review Comment: https://git.openjdk.org/jdk/pull/16358#discussion_r1373595837
PR Review Comment: https://git.openjdk.org/jdk/pull/16358#discussion_r1373537417
PR Review Comment: https://git.openjdk.org/jdk/pull/16358#discussion_r1373607149

Reply via email to