date:20201216

[JDK-8257080] Java does not try all DNS results when opening a socket

2020-12-16 Thread Benjamin Marwell

Hello everyone,

I would like to bring up the enhancement JDK-8257080[1] on this mailing
list.

tl;dr symptoms:
Java will use the first IP address from DNS and will fail to connect when
the target host is offline, even when the other IPs would have worked. This
is also true for the more recent Socket implementations from Java 14
onwards..

Proposed resolution:
Implementation similar to python. Late(r) DNS resolution when the socket is
being opened (i.e. a target port is known), then iterate over IP addresses
returned by DNS and try a socket_open.

Workarounds:
None. It is not possible to write an agent which alters java.net classes,
as they are protected.

I would like to bring this issue to your attention. I reported it using my
work mail at oracle’s bug tracker, but I think other applications might
suffer from this as well.

Best regards,
Ben

[1] https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8257080

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

2020-12-16 Thread Alan Bateman


On 16/12/2020 08:09, Benjamin Marwell wrote:

Hello everyone,

I would like to bring up the enhancement JDK-8257080[1] on this 
mailing list.


tl;dr symptoms:
Java will use the first IP address from DNS and will fail to connect 
when the target host is offline, even when the other IPs would have 
worked. This is also true for the more recent Socket implementations 
from Java 14 onwards..


Proposed resolution:
Implementation similar to python. Late(r) DNS resolution when the 
socket is being opened (i.e. a target port is known), then iterate 
over IP addresses returned by DNS and try a socket_open.


I can't tell if your proposal is focused on the Socket constructors that 
take a hostname or more generally changing all APIs that connect to a 
SocketAddress to support connecting to an isUnresolved 
InetSocketAddress. There is merit into exploring this, as is exploring 
new APIs. Experience with Python and other run-times would be useful. I 
could imagine prototypes that range from a simple API that iterates 
through the IP address to implementations of the Happy Eyeballs RFC and 
other algorithms to improve the user experience when a connection is 
slow to establish. There is synergy between such exploration and other 
efforts, including the ongoing work to add a service provider interface 
for name resolution that will open the door to improved APIs for user 
facing applications.


-Alan

Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v3]

2020-12-16 Thread Andrey Turbanov

> 8258422: Cleanup unnecessary null comparison before instanceof check in 
> java.base

Andrey Turbanov has updated the pull request incrementally with one additional 
commit since the last revision:

  8258422: Cleanup unnecessary null comparison before instanceof check in 
java.base
  use instanceof pattern matching in UnixPath too

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/20/files
  - new: https://git.openjdk.java.net/jdk/pull/20/files/603cd364..0d2ac405

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

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

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

Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v3]

2020-12-16 Thread Chris Hegarty

On Wed, 16 Dec 2020 09:20:09 GMT, Andrey Turbanov 
 wrote:

>> 8258422: Cleanup unnecessary null comparison before instanceof check in 
>> java.base
>
> Andrey Turbanov has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8258422: Cleanup unnecessary null comparison before instanceof check in 
> java.base
>   use instanceof pattern matching in UnixPath too

Let's take advantage of "flow scoping" to eliminate some of these casts. A few 
examples follow.

src/java.base/share/classes/java/net/InetSocketAddress.java line 414:

> 412: if (!(obj instanceof InetSocketAddress))
> 413: return false;
> 414: return holder.equals(((InetSocketAddress) obj).holder);

If we restructure this a little we can get:

public final boolean equals(Object obj) {
if (obj instanceof InetSocketAddress that)
return holder.equals(that.holder);
return false;
}

src/java.base/share/classes/java/net/InetSocketAddress.java line 124:

> 122: if (!(obj instanceof InetSocketAddressHolder))
> 123: return false;
> 124: InetSocketAddressHolder that = (InetSocketAddressHolder)obj;

If we restructure this a little we can take advantage of flow scoping, e.g.

 public final boolean equals(Object obj) {
if (!(obj instanceof InetSocketAddressHolder that))
return false;
boolean sameIP;
if (addr != null)
sameIP = addr.equals(that.addr);
else if (hostname != null)
sameIP = (that.addr == null) &&
hostname.equalsIgnoreCase(that.hostname);
else
sameIP = (that.addr == null) && (that.hostname == null);
return sameIP && (port == that.port);
}

-

Changes requested by chegar (Reviewer).

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

Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v4]

2020-12-16 Thread Andrey Turbanov

> 8258422: Cleanup unnecessary null comparison before instanceof check in 
> java.base

Andrey Turbanov has updated the pull request incrementally with one additional 
commit since the last revision:

  8258422: Cleanup unnecessary null comparison before instanceof check in 
java.base
  take advantage of "flow scoping" to eliminate casts

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/20/files
  - new: https://git.openjdk.java.net/jdk/pull/20/files/0d2ac405..f5727ca9

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=20&range=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=20&range=02-03

  Stats: 42 lines in 12 files changed: 1 ins; 10 del; 31 mod
  Patch: https://git.openjdk.java.net/jdk/pull/20.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/20/head:pull/20

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

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

2020-12-16 Thread Alan Bateman

On 16/12/2020 09:04, Benjamin Marwell wrote:

Hi Alan,

> I can't tell if your proposal is focused on the Socket constructors
> that take a hostname or more generally changing all APIs that
> connect to a SocketAddress to support connecting to an
> isUnresolved InetSocketAddress

Way before we even get to sockets. I am talking about InetAddress.
It’s resolving logic is resolve()[0]. That’s it. But an InetAddress
does not carry any

port information and thus should not resolve in my opinion.

---

I think I would start looking in java/net/AbstractPlainSocketImpl.java.
These are the relevant lines:
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/net/AbstractPlainSocketImpl.java#L149-L164

For the new implementation these lines seem pretty relevant:
https://github.com/openjdk/jdk/blob/51d5164ca2b4801c14466e8d1420ecf27cb7615f/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java#L560-L562

That is where I would probably add the for loop.

Here is python’s implementation as a reference:
https://github.com/python/cpython/blob/e42b705188271da108de42b55d9344642170aa2b/Lib/socket.py#L707-L719

By the way, any changes would solve a few other issues as well:
https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8201428

https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8192780

If I read your mail correctly, I think what you are proposing is to
change the existing APIs to allow an unresolved InetSocketAddress to be
specified, is that right? I suspect this approach will have a lot of
implications for methods that are called after the socket is created as
it would require re-creating the underlying socket when a connect
attempt fails (the socket may have been explicitly bound or socket
options were set). There are issues with the semantics of the timeout
parameter. It might not be possible to even do this with APIs such as
SocketChannel when the channel is configured non-blocking.

If the starting point is the Socket constructor that takes a host name
and port then it might be easier because that can do the lookup and
cycle through the addresses and/or overlap the connection attempts.

If you have cycles to do experiments and report back then it would be
useful. I assume the attractiveness of changing the existing APIs is so
that existing usages, e.g. HTTP protocol handler, could use it without
changing to a new APIs. Maybe changing existing vs. new API could be
part of your investigation? I think it would also be useful to know if a
simple iteration through the addresses is sufficient for more cases or
whether something close to Happy Eyeballs would be needed. There have
been a few discussions here over the years on the latter as it is hard
to get right and there may be a case of having support for that in the APIs.

-Alan

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

2020-12-16 Thread Benjamin Marwell

On 16.12.20 13:49, Alan Bateman wrote:

On 16/12/2020 09:04, Benjamin Marwell wrote:

Hi Alan,

Way before we even get to sockets. I am talking about InetAddress.
It’s resolving logic is resolve()[0]. That’s it. But an InetAddress
does not carry any

port information and thus should not resolve in my opinion.

---

That is where I would probably add the for loop.

Here is python’s implementation as a reference:
https://github.com/python/cpython/blob/e42b705188271da108de42b55d9344642170aa2b/Lib/socket.py#L707-L719

By the way, any changes would solve a few other issues as well:
https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8201428

https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8192780

-Alan

Huh… the APIs have already been changed which is why the issues above
exist. There is no workaround anymore.

It is true that I did not take into account that other methods would
rely on this. The biggest issue I see (easily) is the connect timeout.
It must either be distributed along all connection attempts or per
connection attempt – both would change semantics.

I just looked up Happy Eyeballs, and this comes very close to
what I mean. I like that it implies DNS-RR.

Lets see if I (or anyone else) can come up with a sample implementation.

Thanks,
Ben

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

2020-12-16 Thread Alan Bateman


On 16/12/2020 13:40, Benjamin Marwell wrote:


Huh… the APIs have already been changed which is why the issues above 
exist. There is no workaround anymore.


Can you expand on this as it's not clear what you mean by "the APIs have 
already changed"? The code that you referenced are implementation 
classes, not exposed.


-Alan.

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

2020-12-16 Thread Benjamin Marwell


On 16.12.20 16:12, Alan Bateman wrote:

On 16/12/2020 13:40, Benjamin Marwell wrote:


Huh… the APIs have already been changed which is why the issues above 
exist. There is no workaround anymore.


Can you expand on this as it's not clear what you mean by "the APIs have 
already changed"? The code that you referenced are implementation 
classes, not exposed.


-Alan.


https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8192780

Granted, it is an SPI. But it seemed to have been used.

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

2020-12-16 Thread Alan Bateman


On 16/12/2020 15:21, Benjamin Marwell wrote:

https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8192780

Granted, it is an SPI. But it seemed to have been used.



Aleksej Efimov is working on an InetAddress SPI, I'm sure there will be 
a draft JEP at some point. It has the potential to complement the 
exploration that we discussing here.


-Alan

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

2020-12-16 Thread Aleks Efimov


Hi Benjamin,

As Alan stated I'm working on adding an SPI [1] which will provide a 
possibility to alter how host names and IP addresses are resolved by JDK 
platform.
I believe it would be possible to use this mechanism for addressing 
issue described in JDK-8257080.


Best Regards,
Aleksei

[1] WIP jdk-sandbox branch: 
https://github.com/openjdk/jdk-sandbox/tree/JDK-8244202-nspi-stream-branch



On 16/12/2020 15:42, Alan Bateman wrote:

On 16/12/2020 15:21, Benjamin Marwell wrote:

https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8192780

Granted, it is an SPI. But it seemed to have been used.



Aleksej Efimov is working on an InetAddress SPI, I'm sure there will 
be a draft JEP at some point. It has the potential to complement the 
exploration that we discussing here.


-Alan

Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v3]

2020-12-16 Thread Aleksei Efimov

On Wed, 16 Dec 2020 09:44:37 GMT, Chris Hegarty  wrote:

>> Andrey Turbanov has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8258422: Cleanup unnecessary null comparison before instanceof check in 
>> java.base
>>   use instanceof pattern matching in UnixPath too
>
> Let's take advantage of "flow scoping" to eliminate some of these casts. A 
> few examples follow.

Hi Andrey,

Could you, please, also take a look at `java.net.Socket`:
java/net/Socket.java:if (bindpoint != null && (!(bindpoint instanceof 
InetSocketAddress)))
java/net/Socket.java-throw new 
IllegalArgumentException("Unsupported address type");
And `HttpURLConnection`:
sun/net/www/protocol/http/HttpURLConnection.java:if (a 
!= null && c instanceof HttpURLConnection) {
sun/net/www/protocol/http/HttpURLConnection.java-
((HttpURLConnection)c).setAuthenticator(a);
The following cmd was used to find them: `rgrep -A 1 "= null .* instanceof "`

-

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

Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v3]

2020-12-16 Thread Andrey Turbanov

On Wed, 16 Dec 2020 18:27:35 GMT, Aleksei Efimov  wrote:

>> Let's take advantage of "flow scoping" to eliminate some of these casts. A 
>> few examples follow.
>
> Hi Andrey,
> 
> Could you, please, also take a look at `java.net.Socket`:
> java/net/Socket.java:if (bindpoint != null && (!(bindpoint instanceof 
> InetSocketAddress)))
> java/net/Socket.java-throw new 
> IllegalArgumentException("Unsupported address type");
> And `HttpURLConnection`:
> sun/net/www/protocol/http/HttpURLConnection.java:if 
> (a != null && c instanceof HttpURLConnection) {
> sun/net/www/protocol/http/HttpURLConnection.java-
> ((HttpURLConnection)c).setAuthenticator(a);
> The following cmd was used to find them: `rgrep -A 1 "= null .* instanceof "`

@AlekseiEfimov in both cases removing `null` check will change semantic of the 
code. Comparison is not redundant.

-

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

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

2020-12-16 Thread Bernd Eckenfels

Hello,

A „happy eyeballs“ implementation, not only for Multiple IPv4 addresses but 
also for IPV6/IPv4 Mixed would be a good thing, however since this

a) adds additional connection Timeouts or decreases connection deadlines
b) potentially is stateful

I think it’s not a good idea to enable it for all connections. A new method to 
specify a policy (and allow a new default policy) would probably be better.

I don’t think doing that in the resolving alone would work, since there is also 
a question of parallelity and a endpoint specific state.

Gruss
Bernd
--
http://bernd.eckenfels.net

Von: net-dev  im Auftrag von Aleks Efimov 

Gesendet: Wednesday, December 16, 2020 5:53:02 PM
An: Benjamin Marwell 
Cc: Alan Bateman ; OpenJDK Network Dev list 

Betreff: Re: [JDK-8257080] Java does not try all DNS results when opening a 
socket

Hi Benjamin,

As Alan stated I'm working on adding an SPI [1] which will provide a
possibility to alter how host names and IP addresses are resolved by JDK
platform.
I believe it would be possible to use this mechanism for addressing
issue described in JDK-8257080.

Best Regards,
Aleksei

[1] WIP jdk-sandbox branch:
https://github.com/openjdk/jdk-sandbox/tree/JDK-8244202-nspi-stream-branch

On 16/12/2020 15:42, Alan Bateman wrote:
> On 16/12/2020 15:21, Benjamin Marwell wrote:
>> https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8192780
>>
>> Granted, it is an SPI. But it seemed to have been used.
>>
>
> Aleksej Efimov is working on an InetAddress SPI, I'm sure there will
> be a draft JEP at some point. It has the potential to complement the
> exploration that we discussing here.
>
> -Alan

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

2020-12-16 Thread Simone Bordet

Hi,

On Wed, Dec 16, 2020 at 5:55 PM Aleks Efimov  wrote:
>
> Hi Benjamin,
>
> As Alan stated I'm working on adding an SPI [1] which will provide a
> possibility to alter how host names and IP addresses are resolved by JDK
> platform.
> I believe it would be possible to use this mechanism for addressing
> issue described in JDK-8257080.

Is it hopefully going to be non-blocking?

-- 
Simone Bordet
---
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless.   Victoria Livschitz

[JDK-8257080] Java does not try all DNS results when opening a socket

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v3]

Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v3]

Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v4]

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v3]

Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v3]

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

15 matches

Site Navigation

Mail list logo

Footer information