On Tue, 30 May 2023 22:46:05 GMT, Terry Chow <[email protected]> wrote:
> The PR adds support for setting the keepalive extended socket options on > Windows. The implemented native code uses the SIO_KEEPALIVE_VALS control > code, which is how keepalive values are set on Windows and so there are a few > caveats. > > 1. The keepalive time and keepalive interval must both be set at the same > time. In the implementation, a new SioKeepAlive class and > ExtendedSocketOptions is created to capture this behaviour for Windows only. > Keepalive enablement must also be configured when using SIO_KEEPALIVE_VALS, > and so by default when setting keepalive time and keepalive interval, > keepalive will automatically be enabled. > > [SIO_KEEPALIVE_VALS > doc](https://learn.microsoft.com/en-us/windows/win32/winsock/sio-keepalive-vals) > > 2. It doesn't seem possible to acquire the Keepalive time and keepalive > interval values on Windows. So, the implementation doesn't support acquiring > the values. > > https://stackoverflow.com/questions/14197347/how-to-query-socket-keep-alive-values-using-winsock/14198462#14198462 > https://stackoverflow.com/questions/18391341/reading-sio-keepalive-vals-fields-on-a-windows-socket-for-keepalive-idle-and-in > > 3. Keepalive probes are not supported. On Windows Vista and later, the number > of keep-alive probes is set to 10 and cannot be changed. > > [SIO_KEEPALIVE_VALS > Remarks](https://learn.microsoft.com/en-us/windows/win32/winsock/sio-keepalive-vals#remarks) > > For testing, I've updated the existing keepalive tests. But, since it's not > possible to acquire the keepalive values on Windows to verify, I've > indirectly tested setting the keepalive values by confirming keepalive is > enabled where possible (since keepalive is enabled also when keepalive values > are set). I think you may have missed my comment in the JBS issue where I asked about the semantics of SIO_KEEPALIVE_VALS and whether we could implement TCP_KEEPIDLE and TCP_KEEPINTERVAL on Windows by knowing what the defaults are. The proposal that you have here is a new Windows specific and JDK-specific socket option that requires a lot of thinking about before going that route. ------------- PR Comment: https://git.openjdk.org/jdk/pull/14232#issuecomment-1570766242
