RFR: 8253940: com/sun/jdi/JdwpAttachTest.java failed with "RuntimeException: ERROR: LingeredApp.startApp was able to attach"

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8253940: com/sun/jdi/JdwpAttachTest.java failed with "RuntimeException: ERROR: LingeredApp.startApp was able to attach"

Alex Menkov-3
Failures related to "no route to host" and "cannot bind to address" errors are covered by https://git.openjdk.java.net/jdk/pull/2633

This change fixes failures with listening on IPv6 loopback and attaching to "localhost" (the test connects to some other process which listens on the same port on IPv4 loopback and as a result gets "handshake failed - received >< - expected >JDWP-Handshake<" error)

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

Commit messages:
 - JDK-8253940

Changes: https://git.openjdk.java.net/jdk/pull/2654/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2654&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8253940
  Stats: 18 lines in 1 file changed: 14 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2654.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2654/head:pull/2654

PR: https://git.openjdk.java.net/jdk/pull/2654
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8253940: com/sun/jdi/JdwpAttachTest.java failed with "RuntimeException: ERROR: LingeredApp.startApp was able to attach"

Chris Plummer-2
On Fri, 19 Feb 2021 20:10:14 GMT, Alex Menkov <[hidden email]> wrote:

> Failures related to "no route to host" and "cannot bind to address" errors are covered by https://git.openjdk.java.net/jdk/pull/2633
>
> This change fixes failures with listening on IPv6 loopback and attaching to "localhost" (the test connects to some other process which listens on the same port on IPv4 loopback and as a result gets "handshake failed - received >< - expected >JDWP-Handshake<" error)

test/jdk/com/sun/jdi/JdwpAttachTest.java line 88:

> 86:
> 87:         // by using "localhost" or empty hostname
> 88:         // we should be able to attach to both IPv4 and IPv6 addresses (127.0.0.1 & ::1).

As long as you're touching this comment, start it with "By" and put more of the comment on the first line.

Also, the comment is a bit misleading now. You can attach to both, but you only try the one that is preferred.

test/jdk/com/sun/jdi/JdwpAttachTest.java line 180:

> 178:     // loopback address.
> 179:     // The method checks if the address is safe to test with current network config.
> 180:     private static boolean addressIfSafeToConnectToLocalhost(InetAddress addr) {

Do you really mean to name it "addressIfSafe..." rather than "addressIsSafe..."?

test/jdk/com/sun/jdi/JdwpAttachTest.java line 178:

> 176:     // Attach to localhost tries to connect to both IPv4 and IPv6 loopback addresses.
> 177:     // But sometimes it causes interference with other processes which can listen on the same port but different
> 178:     // loopback address.

Move a few words from the end of the 2nd line of the comment down to the 3rd.

test/jdk/com/sun/jdi/JdwpAttachTest.java line 183:

> 181:         boolean ipv6 = Boolean.parseBoolean(System.getProperty("java.net.preferIPv6Addresses"));
> 182:         return ipv6 == (addr instanceof Inet6Address);
> 183:     }

In the CR you said:

> But JDI framework respects "java.net.preferIPv6Addresses" system property (which by default is false), so connecting to empty host tries IPv4 addresses 1st and tries IPv6 addresses only if connect to IPv4 failed.

Are you referring to the JDI implementation, or the JDI test framework. In any case, I had trouble locating this anywhere in the source.

Also, "preferv6" doesn't mean that's what it ends up using. It just tries v6 first before deferring to v4 if v6 fails (or the opposite if it doesn't prefer v6). I don't think you are capturing that part of the logic. What happens if preferv6 is true but the connect to a v6 port fails. Should the test try to connect to the v4 in that case?

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

PR: https://git.openjdk.java.net/jdk/pull/2654
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8253940: com/sun/jdi/JdwpAttachTest.java failed with "RuntimeException: ERROR: LingeredApp.startApp was able to attach" [v2]

Alex Menkov-3
In reply to this post by Alex Menkov-3
> Failures related to "no route to host" and "cannot bind to address" errors are covered by https://git.openjdk.java.net/jdk/pull/2633
>
> This change fixes failures with listening on IPv6 loopback and attaching to "localhost" (the test connects to some other process which listens on the same port on IPv4 loopback and as a result gets "handshake failed - received >< - expected >JDWP-Handshake<" error)

Alex Menkov has updated the pull request incrementally with one additional commit since the last revision:

  Updated per Chris feedback

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2654/files
  - new: https://git.openjdk.java.net/jdk/pull/2654/files/fe12d328..2cf7031a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2654&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2654&range=00-01

  Stats: 8 lines in 1 file changed: 2 ins; 0 del; 6 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2654.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2654/head:pull/2654

PR: https://git.openjdk.java.net/jdk/pull/2654
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8253940: com/sun/jdi/JdwpAttachTest.java failed with "RuntimeException: ERROR: LingeredApp.startApp was able to attach" [v2]

Alex Menkov-3
In reply to this post by Chris Plummer-2
On Fri, 19 Feb 2021 20:30:19 GMT, Chris Plummer <[hidden email]> wrote:

>> Alex Menkov has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Updated per Chris feedback
>
> test/jdk/com/sun/jdi/JdwpAttachTest.java line 180:
>
>> 178:     // loopback address.
>> 179:     // The method checks if the address is safe to test with current network config.
>> 180:     private static boolean addressIfSafeToConnectToLocalhost(InetAddress addr) {
>
> Do you really mean to name it "addressIfSafe..." rather than "addressIsSafe..."?

Correct. Fixed.

> test/jdk/com/sun/jdi/JdwpAttachTest.java line 178:
>
>> 176:     // Attach to localhost tries to connect to both IPv4 and IPv6 loopback addresses.
>> 177:     // But sometimes it causes interference with other processes which can listen on the same port but different
>> 178:     // loopback address.
>
> Move a few words from the end of the 2nd line of the comment down to the 3rd.

Done.

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

PR: https://git.openjdk.java.net/jdk/pull/2654
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8253940: com/sun/jdi/JdwpAttachTest.java failed with "RuntimeException: ERROR: LingeredApp.startApp was able to attach" [v2]

Alex Menkov-3
In reply to this post by Chris Plummer-2
On Fri, 19 Feb 2021 20:50:58 GMT, Chris Plummer <[hidden email]> wrote:

> In the CR you said:
>
> > But JDI framework respects "java.net.preferIPv6Addresses" system property (which by default is false), so connecting to empty host tries IPv4 addresses 1st and tries IPv6 addresses only if connect to IPv4 failed.
>
> Are you referring to the JDI implementation, or the JDI test framework. In any case, I had trouble locating this anywhere in the source.

This is about JDI implementation (the test uses JDWP agent to attach - corresponding code is at src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c)

> Also, "preferv6" doesn't mean that's what it ends up using. It just tries v6 first before deferring to v4 if v6 fails (or the opposite if it doesn't prefer v6). I don't think you are capturing that part of the logic. What happens if preferv6 is true but the connect to a v6 port fails. Should the test try to connect to the v4 in that case?

Well. Lets consider the case when we have both IPv4 and IPv6 loopback addresses and "preferv6" is true (for "preferv6" == false it works opposite).
The test does:
1. - listen on IPv6 loopback ("::1")
   - tries to attach to localhost
     JDI 1st tries IPv6, which is expected to succeed. If it fails, that means the test should fail too. Theoretically in the case test can successfully attach to some other process which listens on the same port IPv4 loopback, but we expect the test to pass :)
2. - listen on IPv4 loopback (127.0.0.1)
   - tries to attach to localhost.
      JDI 1st tries IPv6, which is expected to fail, but if there is some other process which listens on the same port on IPv6 loopback, we get the error like described in the issue (handshake failure)

So the logic is to test 1 and skip 2

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

PR: https://git.openjdk.java.net/jdk/pull/2654
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8253940: com/sun/jdi/JdwpAttachTest.java failed with "RuntimeException: ERROR: LingeredApp.startApp was able to attach" [v2]

Chris Plummer-2
In reply to this post by Alex Menkov-3
On Fri, 19 Feb 2021 23:04:59 GMT, Alex Menkov <[hidden email]> wrote:

>> Failures related to "no route to host" and "cannot bind to address" errors are covered by https://git.openjdk.java.net/jdk/pull/2633
>>
>> This change fixes failures with listening on IPv6 loopback and attaching to "localhost" (the test connects to some other process which listens on the same port on IPv4 loopback and as a result gets "handshake failed - received >< - expected >JDWP-Handshake<" error)
>
> Alex Menkov has updated the pull request incrementally with one additional commit since the last revision:
>
>   Updated per Chris feedback

Marked as reviewed by cjplummer (Reviewer).

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

PR: https://git.openjdk.java.net/jdk/pull/2654
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8253940: com/sun/jdi/JdwpAttachTest.java failed with "RuntimeException: ERROR: LingeredApp.startApp was able to attach" [v2]

Chris Plummer-2
In reply to this post by Alex Menkov-3
On Fri, 19 Feb 2021 23:54:11 GMT, Alex Menkov <[hidden email]> wrote:

>> test/jdk/com/sun/jdi/JdwpAttachTest.java line 183:
>>
>>> 181:         boolean ipv6 = Boolean.parseBoolean(System.getProperty("java.net.preferIPv6Addresses"));
>>> 182:         return ipv6 == (addr instanceof Inet6Address);
>>> 183:     }
>>
>> In the CR you said:
>>
>>> But JDI framework respects "java.net.preferIPv6Addresses" system property (which by default is false), so connecting to empty host tries IPv4 addresses 1st and tries IPv6 addresses only if connect to IPv4 failed.
>>
>> Are you referring to the JDI implementation, or the JDI test framework. In any case, I had trouble locating this anywhere in the source.
>>
>> Also, "preferv6" doesn't mean that's what it ends up using. It just tries v6 first before deferring to v4 if v6 fails (or the opposite if it doesn't prefer v6). I don't think you are capturing that part of the logic. What happens if preferv6 is true but the connect to a v6 port fails. Should the test try to connect to the v4 in that case?
>
>> In the CR you said:
>>
>> > But JDI framework respects "java.net.preferIPv6Addresses" system property (which by default is false), so connecting to empty host tries IPv4 addresses 1st and tries IPv6 addresses only if connect to IPv4 failed.
>>
>> Are you referring to the JDI implementation, or the JDI test framework. In any case, I had trouble locating this anywhere in the source.
>
> This is about JDI implementation (the test uses JDWP agent to attach - corresponding code is at src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c)
>
>> Also, "preferv6" doesn't mean that's what it ends up using. It just tries v6 first before deferring to v4 if v6 fails (or the opposite if it doesn't prefer v6). I don't think you are capturing that part of the logic. What happens if preferv6 is true but the connect to a v6 port fails. Should the test try to connect to the v4 in that case?
>
> Well. Lets consider the case when we have both IPv4 and IPv6 loopback addresses and "preferv6" is true (for "preferv6" == false it works opposite).
> The test does:
> 1. - listen on IPv6 loopback ("::1")
>    - tries to attach to localhost
>      JDI 1st tries IPv6, which is expected to succeed. If it fails, that means the test should fail too. Theoretically in the case test can successfully attach to some other process which listens on the same port IPv4 loopback, but we expect the test to pass :)
> 2. - listen on IPv4 loopback (127.0.0.1)
>    - tries to attach to localhost.
>       JDI 1st tries IPv6, which is expected to fail, but if there is some other process which listens on the same port on IPv6 loopback, we get the error like described in the issue (handshake failure)
>
> So the logic is to test 1 and skip 2

Ok, that makes sense. Thanks

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

PR: https://git.openjdk.java.net/jdk/pull/2654
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8253940: com/sun/jdi/JdwpAttachTest.java failed with "RuntimeException: ERROR: LingeredApp.startApp was able to attach" [v2]

Alex Menkov-3
In reply to this post by Chris Plummer-2
On Sat, 20 Feb 2021 03:05:55 GMT, Chris Plummer <[hidden email]> wrote:

>> Alex Menkov has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Updated per Chris feedback
>
> Marked as reviewed by cjplummer (Reviewer).

Ping. need 2nd reviewer.

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

PR: https://git.openjdk.java.net/jdk/pull/2654
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8253940: com/sun/jdi/JdwpAttachTest.java failed with "RuntimeException: ERROR: LingeredApp.startApp was able to attach" [v2]

Leonid Mesnik-2
In reply to this post by Alex Menkov-3
On Fri, 19 Feb 2021 23:04:59 GMT, Alex Menkov <[hidden email]> wrote:

>> Failures related to "no route to host" and "cannot bind to address" errors are covered by https://git.openjdk.java.net/jdk/pull/2633
>>
>> This change fixes failures with listening on IPv6 loopback and attaching to "localhost" (the test connects to some other process which listens on the same port on IPv4 loopback and as a result gets "handshake failed - received >< - expected >JDWP-Handshake<" error)
>
> Alex Menkov has updated the pull request incrementally with one additional commit since the last revision:
>
>   Updated per Chris feedback

Marked as reviewed by lmesnik (Committer).

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

PR: https://git.openjdk.java.net/jdk/pull/2654