classification
Title: Add macOS TCP_KEEPALIVE to available socket options
Type: enhancement Stage: resolved
Components: Extension Modules Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ShaneHarvey, llawall, malin, miss-islington, pablogsal, steve.dower
Priority: normal Keywords: patch

Created on 2018-10-08 13:38 by llawall, last changed 2021-07-14 23:15 by miss-islington. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 9759 closed python-dev, 2018-10-08 13:42
PR 25079 merged ShaneHarvey, 2021-03-29 21:08
PR 27153 merged miss-islington, 2021-07-14 22:53
Messages (8)
msg327351 - (view) Author: Robert Wall (llawall) Date: 2018-10-08 13:38
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.
msg327441 - (view) Author: Robert Wall (llawall) Date: 2018-10-09 21:58
Acknowledging the test failure and message pointing to #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 #32394. Any thoughts on this one?
msg327450 - (view) Author: Ma Lin (malin) * Date: 2018-10-10 01:53
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
msg327656 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2018-10-13 16:10
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.
msg327782 - (view) Author: Robert Wall (llawall) Date: 2018-10-15 20:31
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 #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
msg393911 - (view) Author: Shane Harvey (ShaneHarvey) * Date: 2021-05-18 22:40
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 https://github.com/python/cpython/pull/25079
msg397515 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-07-14 22:53
New changeset d59d7374a364c4e5c2b9a83d8e4543ee494285b8 by Shane Harvey in branch 'main':
bpo-34932: Add socket.TCP_KEEPALIVE for macOS (GH-25079)
https://github.com/python/cpython/commit/d59d7374a364c4e5c2b9a83d8e4543ee494285b8
msg397517 - (view) Author: miss-islington (miss-islington) Date: 2021-07-14 23:15
New changeset ff7af2203c1f03d9300e93e3e06a47fb78cc2bef by Miss Islington (bot) in branch '3.10':
bpo-34932: Add socket.TCP_KEEPALIVE for macOS (GH-25079)
https://github.com/python/cpython/commit/ff7af2203c1f03d9300e93e3e06a47fb78cc2bef
History
Date User Action Args
2021-07-14 23:15:40miss-islingtonsetmessages: + msg397517
2021-07-14 22:54:49pablogsalsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-07-14 22:53:24miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request25691
2021-07-14 22:53:22pablogsalsetnosy: + pablogsal
messages: + msg397515
2021-05-18 22:40:14ShaneHarveysetmessages: + msg393911
2021-03-29 21:08:44ShaneHarveysetnosy: + ShaneHarvey
pull_requests: + pull_request23829
2018-10-15 20:31:49llawallsetmessages: + msg327782
2018-10-13 16:10:48steve.dowersetmessages: + msg327656
2018-10-10 01:53:47malinsetnosy: + malin
messages: + msg327450
2018-10-09 21:58:19llawallsetversions: + Python 3.6, Python 3.7
nosy: + steve.dower

messages: + msg327441

components: + Extension Modules, - Library (Lib)
2018-10-08 13:42:30python-devsetkeywords: + patch
stage: patch review
pull_requests: + pull_request9144
2018-10-08 13:38:10llawallcreate