On Mon, 26 Jan 2026 12:50:29 GMT, Jaikiran Pai <[email protected]> wrote:

>> Can I please get a review of this change which proposes to remove the 
>> confusing "getsockopt" text from the exception reported on Windows when 
>> `SocketChannel.finishConnect()` raises an exception due to a failed 
>> `connect()`? 
>> 
>> This addresses https://bugs.openjdk.org/browse/JDK-8376290 and as noted in 
>> that issue, this looks like an oversight when changes were done for 
>> https://bugs.openjdk.org/browse/JDK-8317603. Prior to that, on older 
>> versions the exception reported on Windows was:
>> 
>> 
>> java.net.ConnectException: Connection refused: no further information
>> 
>> 
>> With the current proposed changes in this PR, the exception will now be 
>> `java.net.ConnectException: Connection refused` which matches what we report 
>> on *nix platforms.
>> 
>> A new jtreg test has been added to reproduce the original text and verify 
>> the change. tier1, tier2 and tier3 continue to pass with this change.
>
> Jaikiran Pai has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   skip the test if the SocketChannel unexpectedly connects to the destination 
> address

src/java.base/unix/native/libnet/net_util_md.c line 78:

> 76:             jio_snprintf(fullMsg, sizeof(fullMsg), "socket closed: %s", 
> msg);
> 77:             JNU_ThrowByName(env, JNU_JAVANETPKG "SocketException", 
> fullMsg);
> 78:         }

For for this PR but at some point I'd like to know if the EBADF handling is 
dead code. It shouldn't happen, at least not with the usages in 
sun.nio.ch/libnio but maybe there is something that I can't think of.

test/jdk/java/nio/channels/Selector/ConnectionRefusedMessage.java line 90:

> 88:                         // this test checks the exception message of a 
> ConnectException, so it's
> 89:                         // OK to skip the test if something unexpectedly 
> accepted the connection
> 90:                         throw new TestAbortedException("unexpectedly 
> connected to " + destAddr);

Could this use the assumptions API too? Only asking because we've now got a mix 
of assumptions API and TestAbortedException.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29409#discussion_r2727572402
PR Review Comment: https://git.openjdk.org/jdk/pull/29409#discussion_r2727576573

Reply via email to