RFR: 8308593: Add Keepalive Extended Socket Options Support for Windows

[Terry Chow]

On Tue, 30 May 2023 22:46:05 GMT, Terry Chow <duke at openjdk.org> 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).

Apologies, I was talking to my liaison on what's the appropriate way to communicate (as I don't have a JBS account). Let me know if it's alright to discuss things here. Thanks for the quick reply on your thoughts.

>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.

I had similar thoughts as well, that this implementation was more of a "last resort". I've had other ideas of ways to go about this that I'll mention below, but there were some contentions and so this implementation was the most straightforward to do to get something working in order to have open dialog. But yeah, absolutely agree on more discussion.

>It would be useful to know if the semantics of the SIO_KEEPALIVE_VALS control code is the equivalent of the TCP_KEEPIDLE + TCP_KEEPINTERVAL on other platforms.

Yes, that's correct. SIO_KEEPALIVE_VALS is the Windows equivalent of setting the keep alive values on a per socket basis (only difference is that both are set at the same time).

> It would also be useful to know if Windows has a system-wide default value for both the keep alive idle and interval. If it does then it might be possible to use the existing extended options, which if under the covers that both need to be set at the same time.

So, the system-wide default keep alive idle and interval for Windows is 2hrs and 1s respectively. I also thought of using default values, but the issue is this:

defaultKeepAliveTime = 2hrs
defaultKeepAliveInterval = 1s

1. setKeepAliveTime(1hr)                       // user sets the keepAliveTime

2. setKeepAlive(1hr, defaultKeepAliveInterval) // under the covers we set both values

3. setKeepAliveInterval(20s)                   // user later sets the interval time

4. setKeepAlive(defaultKeepAliveTime, 20s)     // prior keepAliveTime of 1hr is now back to default


But we can get around this by keeping track of the current set keepAlive values.

defaultKeepAliveTime = 2hrs
defaultKeepAliveInterval = 1s
currentKeepAliveTime = defaultKeepAliveTime 
currentKeepAliveInterval = defaultKeepAliveInterval

1. setKeepAliveTime(1hr)                       // user sets the keepAliveTime

2. setKeepAlive(1hr, currentKeepAliveInterval) // under the covers we set both values, keep track of new keepAliveTime value
   currentKeepAliveTime = 1hr

3. setKeepAliveInterval(20s)                   // user later sets the interval time

4. setKeepAlive(currentKeepAliveTime, 20s)     // use prior set keepAliveTime
   currentKeepAliveInterval  = 20s


However, we can't keep track of the current keep alive values in `WindowsSocketOptions` as the same instance is shared by all socket instances. The place that makes the most sense to keep track of the current keep alive values seems like at the socket class level. What are your thoughts on keeping track of the values at the socket level? I was hesistant on this route because it doesn't seem right to track Windows only keep alive values at the socket class level.

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

PR Comment: https://git.openjdk.org/jdk/pull/14232#issuecomment-1572442618