From 572636d42566da8eb6e85d5b8164e9ed8860a255 Mon Sep 17 00:00:00 2001 From: Yury Selivanov Date: Fri, 15 Dec 2017 21:53:08 -0500 Subject: [PATCH] bpo-27456: Ensure TCP_NODELAY is set on linux (#4231) (#4898) (cherry picked from commit e796b2fe26f220107ac50667de6cc86c82b465e3) --- Lib/asyncio/base_events.py | 43 ++++++++++--------- Lib/asyncio/selector_events.py | 2 +- Lib/asyncio/unix_events.py | 4 +- Lib/test/test_asyncio/test_base_events.py | 7 --- Lib/test/test_asyncio/test_selector_events.py | 27 ++++++++++++ .../2017-11-02-11-57-41.bpo-27456.snzyTC.rst | 1 + 6 files changed, 53 insertions(+), 31 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2017-11-02-11-57-41.bpo-27456.snzyTC.rst diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py index 8cc655c79f8..90757db41c6 100644 --- a/Lib/asyncio/base_events.py +++ b/Lib/asyncio/base_events.py @@ -84,18 +84,24 @@ def _set_reuseport(sock): 'SO_REUSEPORT defined but not implemented.') -def _is_stream_socket(sock): - # Linux's socket.type is a bitmask that can include extra info - # about socket, therefore we can't do simple - # `sock_type == socket.SOCK_STREAM`. - return (sock.type & socket.SOCK_STREAM) == socket.SOCK_STREAM +def _is_stream_socket(sock_type): + if hasattr(socket, 'SOCK_NONBLOCK'): + # Linux's socket.type is a bitmask that can include extra info + # about socket (like SOCK_NONBLOCK bit), therefore we can't do simple + # `sock_type == socket.SOCK_STREAM`, see + # https://github.com/torvalds/linux/blob/v4.13/include/linux/net.h#L77 + # for more details. + return (sock_type & 0xF) == socket.SOCK_STREAM + else: + return sock_type == socket.SOCK_STREAM -def _is_dgram_socket(sock): - # Linux's socket.type is a bitmask that can include extra info - # about socket, therefore we can't do simple - # `sock_type == socket.SOCK_DGRAM`. - return (sock.type & socket.SOCK_DGRAM) == socket.SOCK_DGRAM +def _is_dgram_socket(sock_type): + if hasattr(socket, 'SOCK_NONBLOCK'): + # See the comment in `_is_stream_socket`. + return (sock_type & 0xF) == socket.SOCK_DGRAM + else: + return sock_type == socket.SOCK_DGRAM def _ipaddr_info(host, port, family, type, proto): @@ -108,14 +114,9 @@ def _ipaddr_info(host, port, family, type, proto): host is None: return None - if type == socket.SOCK_STREAM: - # Linux only: - # getaddrinfo() can raise when socket.type is a bit mask. - # So if socket.type is a bit mask of SOCK_STREAM, and say - # SOCK_NONBLOCK, we simply return None, which will trigger - # a call to getaddrinfo() letting it process this request. + if _is_stream_socket(type): proto = socket.IPPROTO_TCP - elif type == socket.SOCK_DGRAM: + elif _is_dgram_socket(type): proto = socket.IPPROTO_UDP else: return None @@ -789,7 +790,7 @@ class BaseEventLoop(events.AbstractEventLoop): if sock is None: raise ValueError( 'host and port was not specified and no sock specified') - if not _is_stream_socket(sock): + if not _is_stream_socket(sock.type): # We allow AF_INET, AF_INET6, AF_UNIX as long as they # are SOCK_STREAM. # We support passing AF_UNIX sockets even though we have @@ -841,7 +842,7 @@ class BaseEventLoop(events.AbstractEventLoop): allow_broadcast=None, sock=None): """Create datagram connection.""" if sock is not None: - if not _is_dgram_socket(sock): + if not _is_dgram_socket(sock.type): raise ValueError( 'A UDP Socket was expected, got {!r}'.format(sock)) if (local_addr or remote_addr or @@ -1054,7 +1055,7 @@ class BaseEventLoop(events.AbstractEventLoop): else: if sock is None: raise ValueError('Neither host/port nor sock were specified') - if not _is_stream_socket(sock): + if not _is_stream_socket(sock.type): raise ValueError( 'A Stream Socket was expected, got {!r}'.format(sock)) sockets = [sock] @@ -1078,7 +1079,7 @@ class BaseEventLoop(events.AbstractEventLoop): This method is a coroutine. When completed, the coroutine returns a (transport, protocol) pair. """ - if not _is_stream_socket(sock): + if not _is_stream_socket(sock.type): raise ValueError( 'A Stream Socket was expected, got {!r}'.format(sock)) diff --git a/Lib/asyncio/selector_events.py b/Lib/asyncio/selector_events.py index 942b627a1c1..aa65702f2db 100644 --- a/Lib/asyncio/selector_events.py +++ b/Lib/asyncio/selector_events.py @@ -43,7 +43,7 @@ def _test_selector_event(selector, fd, event): if hasattr(socket, 'TCP_NODELAY'): def _set_nodelay(sock): if (sock.family in {socket.AF_INET, socket.AF_INET6} and - sock.type == socket.SOCK_STREAM and + base_events._is_stream_socket(sock.type) and sock.proto == socket.IPPROTO_TCP): sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1) else: diff --git a/Lib/asyncio/unix_events.py b/Lib/asyncio/unix_events.py index 2806ea8dc90..5ea6c20c674 100644 --- a/Lib/asyncio/unix_events.py +++ b/Lib/asyncio/unix_events.py @@ -242,7 +242,7 @@ class _UnixSelectorEventLoop(selector_events.BaseSelectorEventLoop): if sock is None: raise ValueError('no path and sock were specified') if (sock.family != socket.AF_UNIX or - not base_events._is_stream_socket(sock)): + not base_events._is_stream_socket(sock.type)): raise ValueError( 'A UNIX Domain Stream Socket was expected, got {!r}' .format(sock)) @@ -297,7 +297,7 @@ class _UnixSelectorEventLoop(selector_events.BaseSelectorEventLoop): 'path was not specified, and no sock specified') if (sock.family != socket.AF_UNIX or - not base_events._is_stream_socket(sock)): + not base_events._is_stream_socket(sock.type)): raise ValueError( 'A UNIX Domain Stream Socket was expected, got {!r}' .format(sock)) diff --git a/Lib/test/test_asyncio/test_base_events.py b/Lib/test/test_asyncio/test_base_events.py index 3f1ec651742..830f0d84a9d 100644 --- a/Lib/test/test_asyncio/test_base_events.py +++ b/Lib/test/test_asyncio/test_base_events.py @@ -116,13 +116,6 @@ class BaseEventTests(test_utils.TestCase): self.assertIsNone( base_events._ipaddr_info('::3%lo0', 1, INET6, STREAM, TCP)) - if hasattr(socket, 'SOCK_NONBLOCK'): - self.assertEqual( - None, - base_events._ipaddr_info( - '1.2.3.4', 1, INET, STREAM | socket.SOCK_NONBLOCK, TCP)) - - def test_port_parameter_types(self): # Test obscure kinds of arguments for "port". INET = socket.AF_INET diff --git a/Lib/test/test_asyncio/test_selector_events.py b/Lib/test/test_asyncio/test_selector_events.py index 3477573ec3e..830b15c0893 100644 --- a/Lib/test/test_asyncio/test_selector_events.py +++ b/Lib/test/test_asyncio/test_selector_events.py @@ -17,6 +17,7 @@ from asyncio.selector_events import _SelectorTransport from asyncio.selector_events import _SelectorSslTransport from asyncio.selector_events import _SelectorSocketTransport from asyncio.selector_events import _SelectorDatagramTransport +from asyncio.selector_events import _set_nodelay MOCK_ANY = mock.ANY @@ -1829,5 +1830,31 @@ class SelectorDatagramTransportTests(test_utils.TestCase): 'Fatal error on transport\nprotocol:.*\ntransport:.*'), exc_info=(ConnectionRefusedError, MOCK_ANY, MOCK_ANY)) + +class TestSelectorUtils(test_utils.TestCase): + def check_set_nodelay(self, sock): + opt = sock.getsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY) + self.assertFalse(opt) + + _set_nodelay(sock) + + opt = sock.getsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY) + self.assertTrue(opt) + + @unittest.skipUnless(hasattr(socket, 'TCP_NODELAY'), + 'need socket.TCP_NODELAY') + def test_set_nodelay(self): + sock = socket.socket(family=socket.AF_INET, type=socket.SOCK_STREAM, + proto=socket.IPPROTO_TCP) + with sock: + self.check_set_nodelay(sock) + + sock = socket.socket(family=socket.AF_INET, type=socket.SOCK_STREAM, + proto=socket.IPPROTO_TCP) + with sock: + sock.setblocking(False) + self.check_set_nodelay(sock) + + if __name__ == '__main__': unittest.main() diff --git a/Misc/NEWS.d/next/Library/2017-11-02-11-57-41.bpo-27456.snzyTC.rst b/Misc/NEWS.d/next/Library/2017-11-02-11-57-41.bpo-27456.snzyTC.rst new file mode 100644 index 00000000000..fa7b5616bb3 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-11-02-11-57-41.bpo-27456.snzyTC.rst @@ -0,0 +1 @@ +Ensure TCP_NODELAY is set on Linux. Tests by Victor Stinner.