Add macOS TCP_KEEPALIVE to available socket options

[llawall]

BPO	34932
Nosy	@llawall, @zooba, @animalize, @ShaneHarvey, @pablogsal, @miss-islington
PRs	bpo-34932: Add TCP_KEEPALIVE to available flags when available #9759 bpo-34932: Add socket.TCP_KEEPALIVE for macOS #25079 [3.10] bpo-34932: Add socket.TCP_KEEPALIVE for macOS (GH-25079) #27153

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2021-07-14.22:54:49.442>
created_at = <Date 2018-10-08.13:38:10.394>
labels = ['extension-modules', '3.8', 'type-feature', '3.7']
title = 'Add macOS TCP_KEEPALIVE to available socket options'
updated_at = <Date 2021-07-14.23:15:40.022>
user = 'https://github.com/llawall'

bugs.python.org fields:

activity = <Date 2021-07-14.23:15:40.022>
actor = 'miss-islington'
assignee = 'none'
closed = True
closed_date = <Date 2021-07-14.22:54:49.442>
closer = 'pablogsal'
components = ['Extension Modules']
creation = <Date 2018-10-08.13:38:10.394>
creator = 'llawall'
dependencies = []
files = []
hgrepos = []
issue_num = 34932
keywords = ['patch']
message_count = 8.0
messages = ['327351', '327441', '327450', '327656', '327782', '393911', '397515', '397517']
nosy_count = 6.0
nosy_names = ['llawall', 'steve.dower', 'malin', 'ShaneHarvey', 'pablogsal', 'miss-islington']
pr_nums = ['9759', '25079', '27153']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue34932'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

[llawall]

macOS uses TCP_KEEPALIVE in place of TCP_KEEPIDLE. It would be good to have this available in the socket library to use directly.

Pull request coming up.

[llawall]

Acknowledging the test failure and message pointing to bpo-32394:

======================================================================
FAIL: test_new_tcp_flags (test.test_socket.TestMSWindowsTCPFlags)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "C:\projects\cpython\lib\test\test_socket.py", line 5992, in test_new_tcp_flags
    self.assertEqual([], unknown,
AssertionError: Lists differ: [] != ['TCP_KEEPALIVE']
Second list contains 1 additional elements.
First extra element 0:
'TCP_KEEPALIVE'
- []
+ ['TCP_KEEPALIVE'] : New TCP flags were discovered. See bpo-32394 for more information

---

It appears that TCP_KEEPALIVE is defined in Windows (in ws2ipdef.h) despite not being documented on https://docs.microsoft.com/en-us/windows/desktop/winsock/ipproto-tcp-socket-options

Given that TCP_KEEPIDLE is #define as the value of TCP_KEEPALIVE, I'd guess that it was added at the same time in which case it probably ought to be added to win_runtime_flags for exclusion based on the same Windows version 1709 too.

steve.dower: I've added you to the nosy list based on your active participation in bpo-32394. Any thoughts on this one?

[animalize]

I searched on Google, an article [1] said: Of note, Mac OS X defines keepidle and keepintvl in terms of milliseconds as opposed to Linux using seconds.

On Windows, it's also seconds. Will this option make trouble for cross-platform programs?

[1] http://coryklein.com/tcp/2015/11/25/custom-configuration-of-tcp-socket-keep-alive-timeouts.html

[zooba]

Yes, looks like TCP_KEEPIDLE is only available from 1709, so it should get the same handling as the TCP_KEEPCNT. If TCP_KEEPALIVE is just a synonym, then it applies for both.

[llawall]

I saw that article too but reading man tcp(4) [1], it says that this value is in seconds for macOS and testing appears to confirm this.

Where the unit of time is different, however, is with the old and new Windows keepalive options discussed in bpo-32394. The SIO_KEEPALIVE_VALS set via the WinSock ioctl is in milliseconds [2] whereas the newer TCP_KEEP* flags are set in seconds [3] but this doesn't seem too bad as they are different interfaces.

I'll add the tcp_keepalive flag to the same win_runtime_flags structure as the other options with minimum version 1709 and update the failing test accordingly to take account of the new option.

[1] https://github.com/apple/darwin-xnu/blob/xnu-4570.1.46/bsd/man/man4/tcp.4#L172
[2] https://msdn.microsoft.com/en-us/library/windows/desktop/dd877220(v=vs.85).aspx
[3] https://docs.microsoft.com/en-us/windows/desktop/winsock/ipproto-tcp-socket-options

[ShaneHarvey]

I opened a PR to add the socket.TCP_KEEPALIVE flag *on macOS only*. I don't see a reason to add it on Windows since as far as I can tell, TCP_KEEPALIVE is completely undocumented there. Besides there are already two ways to configure keepalive times on Windows (using sock.ioctl+SIO_KEEPALIVE_VALS and using sock.setsockopt+TCP_KEEPIDLE), I don't think we should add a third way.

See #25079

[pablogsal]

New changeset d59d737 by Shane Harvey in branch 'main':
bpo-34932: Add socket.TCP_KEEPALIVE for macOS (GH-25079)
d59d737

[miss-islington]

New changeset ff7af22 by Miss Islington (bot) in branch '3.10':
bpo-34932: Add socket.TCP_KEEPALIVE for macOS (GH-25079)
ff7af22