RFR: 8343791: Socket.connect API should document whether the socket will be closed when hostname resolution fails or another error occurs [v3]

[Daniel Fuchs]

On Tue, 19 Nov 2024 17:21:10 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> src/java.base/share/classes/java/net/Socket.java line 706:
>> 
>>> 704:         try {
>>> 705:             getImpl().connect(epoint, timeout);
>>> 706:         } catch (IOException error) {
>> 
>> At L680 we reject an endpoint that is not an InetSocketAddresss. I think we should follow this with a check to ensure that the address is resolved, that way Socket does all the validation before delegation to the SocketImpl. It means if getImpl().connect(..) throws any exception or error then you can close the Socket, meaning you can catch Throwable.
>
> The spec is still not right: we throw SocketException if the socket is already connected, but we do not close the socket in that case.

By comparison SocketChannel throws `java.nio.channels.AlreadyConnectedException` which is a subclass of IllegalSstateException (a runtime exception) and leaves the socket open. I am not sure we could change `Socket::connect` to throw a different exception than `SocketException` in that case due to compatibility reasons. So we now have to choose between either:

- modify the spec to explicitely say that the socket remains open if a SocketException is thrown because the socket is already connected
- modify the behavior to also close the socket in that case

Whatever alternative we choose should be also listed in the CSR.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1848779024