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

[Alan Bateman]

On Wed, 20 Nov 2024 12:42:13 GMT, Volkan Yazıcı <duke at openjdk.org> wrote:

>> src/java.base/share/classes/java/net/Socket.java line 709:
>> 
>>> 707:         }
>>> 708: 
>>> 709:         try {
>> 
>> At (old) L680, it checks that endpoint is an InetSocketAddress. We should also check that it resolved, as in:
>> 
>>         if (epoint.isUnresolved()) {
>>             close();
>>             throw new UnknownHostException(epoint.getHostName() + " is unresolved");
>>         }
>> 
>> The benefit is the the constructor does the validation before is delegates to the SocketImpl. We could change the SocketImpl to throw IAE for this case, it doesn't matter because it will never happen if Socket rejects it.
>
> Implemented in 424fc8ccd3a495a49f859f0cfc563cf463b3efeb, along with a test. Are you sure we want to have a `close()` there instead of `closeQuietly()`?

I can't think of any cases where close could throw here but it would be better to catch any exception or error from close so that it doesn't override the UHE. A supporting method to catch and add as a suppressed exception is okay but maybe think more about the name as "quietly" it misleading, and "parentError" is a bit strange as it's not an Error.

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

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