On 16.12.20 13:49, Alan Bateman wrote:
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
<https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/net/AbstractPlainSocketImpl.java*L149-L164__;Iw!!GqivPVa7Brio!IQGo98vib-LqfZaVN1Y7MJ51_ZRRrljY_n6IM-btaxHD3suFHe77kiBgTeSYGhb8tw$>
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
<https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/51d5164ca2b4801c14466e8d1420ecf27cb7615f/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java*L560-L562__;Iw!!GqivPVa7Brio!IQGo98vib-LqfZaVN1Y7MJ51_ZRRrljY_n6IM-btaxHD3suFHe77kiBgTeQy8PBhSg$>
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
<https://urldefense.com/v3/__https://github.com/python/cpython/blob/e42b705188271da108de42b55d9344642170aa2b/Lib/socket.py*L707-L719__;Iw!!GqivPVa7Brio!IQGo98vib-LqfZaVN1Y7MJ51_ZRRrljY_n6IM-btaxHD3suFHe77kiBgTeTvZM9G2A$>
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://urldefense.com/v3/__https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8201428__;!!GqivPVa7Brio!IQGo98vib-LqfZaVN1Y7MJ51_ZRRrljY_n6IM-btaxHD3suFHe77kiBgTeQkc7Ex1w$>
https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8192780
<https://urldefense.com/v3/__https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8192780__;!!GqivPVa7Brio!IQGo98vib-LqfZaVN1Y7MJ51_ZRRrljY_n6IM-btaxHD3suFHe77kiBgTeSpuxbIvA$>
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
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
