RFR: 8308593: Add Keepalive Extended Socket Options Support for Windows [v2]
Jaikiran Pai
jpai at openjdk.org
Wed Jul 19 12:03:45 UTC 2023
On Fri, 16 Jun 2023 17:19:28 GMT, Terry Chow <duke at openjdk.org> wrote:
>> The PR adds support for the keepalive extended socket options on Windows. For TCP_KEEPIDLE and TCP_KEEPINTVL, these options are supported starting from Windows 10 version 1709. TCP_KEEPCNT is supported starting from Windows 10 version 1703. Information on these socket options can be found [here](https://learn.microsoft.com/en-us/windows/win32/winsock/ipproto-tcp-socket-options).
>>
>> I've also corrected the `handleError()` function. On Windows, the error needs to be retrieved using `WSAGetLastError()` and error codes are prefixed with "WSA". Information on this can be found [here](https://learn.microsoft.com/en-us/windows/win32/winsock/error-codes-errno-h-errno-and-wsagetlasterror-2).
>>
>>>The error codes returned by Windows Sockets are similar to UNIX socket error code constants, but the constants are all prefixed with WSA.
>>
>>>Error codes set by Windows Sockets are not made available through the errno variable.
>>
>> No new tests were added as the existing tests should cover this.
>
> Terry Chow has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains one additional commit since the last revision:
>
> Support keepalive extended socket options for Windows
The changes to `WindowsSocketOptions.java` which are merely wiring changes, look OK to me. I don't have experience with Windows nor much JNI code, so I'll let other reviewers take a look.
I see that the JNI code is updated to use `WSAGetLastError()` instead of `errno` to get the error code. You already provided a link to the documentation of that API and reading through it, it appears to be the right thing to do and it seems to be supported on all relevant Windows versions of interest. However, that change does impact other existing code in that file. Do you think we could do that change in a separate PR?
In the meantime, could you please update the copyright year on both these changed files to `2022, 2023, ` from the current `2022, `?
-------------
PR Comment: https://git.openjdk.org/jdk/pull/14232#issuecomment-1641955879
More information about the net-dev
mailing list