RFR: JDK-8317603: provide handleSocketErrorWithMessage in windows Net.c for extended exception messages [v3]

[Alan Bateman]

On Fri, 6 Oct 2023 11:08:13 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:

>> On Windows, we miss a handleSocketErrorWithMessage function that provides an additional message showing what went wrong in the Net.c coding.  On Unix we have this function. 
>> This leads sometimes to exceptions like
>> 
>> MSG RTE: javax.naming.CommunicationException: example.com:1234 [Root exception is java.net.ConnectException: Connection timed out: no further information]
>> 
>> see https://bugs.openjdk.org/browse/JDK-8317307
>> 
>> It would be better to have a message explaining the reason instead of "no further information" .
>
> Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision:
> 
>   adjust message texts

Moving to using NET_ThrowNew is okay but it creates inconsistencies that have to be fixed. I think this PR will go through a few iterations.

Some background here is that the socket code no longer exists in libnet/net.dll and at some point we need to do some cleanup and refactoring so that libnio does not have libnet as a dependency (if you go through the JBS issue you'll see a number of startup/first-use issues caused by this dependency). We don't have to do this as part of this PR of course but it provides some context.

For this PR, the issue with changing the specific functions to use NET_ThrowNet is that handleSocketError is left in place for DatagramChannelImpl.c, IOUtil.c and UnixDomainSocket.c. We shouldn't do that. We either replace usages (and remove handleSocketError) or change the existing function to take an additional message.

src/java.base/windows/native/libnio/ch/Net.c line 559:

> 557:     int n = setGroupSourceReqOption(env, fdo, opt, group, index, source);
> 558:     if (n == SOCKET_ERROR) {
> 559:         NET_ThrowNew(env, WSAGetLastError(), "setsockopt with group source request");

That message looks odd, I think you want "setsocketopt to block or unblock source".

src/java.base/windows/native/libnio/ch/Net.c line 638:

> 636:     int count = 0;
> 637:     if (NET_SocketAvailable(fdval(env, fdo), &count) != 0) {
> 638:         NET_ThrowNew(env, WSAGetLastError(), "socket availability check");

This should be "ioctlsocket", not ""socket availability check". Better still would be to just replace the usage of NET_SocketAvailable to use ioctlsocket directly as we will eventually remove NET_SocketAvailable.

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

PR Comment: https://git.openjdk.org/jdk/pull/16057#issuecomment-1751703045
PR Review Comment: https://git.openjdk.org/jdk/pull/16057#discussion_r1349516548
PR Review Comment: https://git.openjdk.org/jdk/pull/16057#discussion_r1349516493