[Python-checkins] bpo-32331: Fix socket.type when SOCK_NONBLOCK is available (#4877)
Yury Selivanov
webhook-mailer at python.org
Mon Dec 18 20:03:01 EST 2017
https://github.com/python/cpython/commit/9818142b1bd20243733a953fb8aa2c7be314c47c
commit: 9818142b1bd20243733a953fb8aa2c7be314c47c
branch: master
author: Yury Selivanov <yury at magic.io>
committer: GitHub <noreply at github.com>
date: 2017-12-18T20:02:54-05:00
summary:
bpo-32331: Fix socket.type when SOCK_NONBLOCK is available (#4877)
files:
A Misc/NEWS.d/next/Library/2017-12-15-23-48-43.bpo-32331.fIg1Uc.rst
M Doc/library/socket.rst
M Doc/whatsnew/3.7.rst
M Lib/socket.py
M Lib/test/test_asyncore.py
M Lib/test/test_socket.py
M Modules/socketmodule.c
diff --git a/Doc/library/socket.rst b/Doc/library/socket.rst
index 42fd7ea0f0b..db032ca7f1f 100644
--- a/Doc/library/socket.rst
+++ b/Doc/library/socket.rst
@@ -482,6 +482,20 @@ The following functions all create :ref:`socket objects <socket-objects>`.
.. versionchanged:: 3.7
The CAN_ISOTP protocol was added.
+ .. versionchanged:: 3.7
+ When :const:`SOCK_NONBLOCK` or :const:`SOCK_CLOEXEC`
+ bit flags are applied to *type* they are cleared, and
+ :attr:`socket.type` will not reflect them. They are still passed
+ to the underlying system `socket()` call. Therefore::
+
+ sock = socket.socket(
+ socket.AF_INET,
+ socket.SOCK_STREAM | socket.SOCK_NONBLOCK)
+
+ will still create a non-blocking socket on OSes that support
+ ``SOCK_NONBLOCK``, but ``sock.type`` will be set to
+ ``socket.SOCK_STREAM``.
+
.. function:: socketpair([family[, type[, proto]]])
Build a pair of connected socket objects using the given address family, socket
@@ -1417,6 +1431,10 @@ to sockets.
* ``sock.setblocking(False)`` is equivalent to ``sock.settimeout(0.0)``
+ .. versionchanged:: 3.7
+ The method no longer applies :const:`SOCK_NONBLOCK` flag on
+ :attr:`socket.type`.
+
.. method:: socket.settimeout(value)
@@ -1429,6 +1447,10 @@ to sockets.
For further information, please consult the :ref:`notes on socket timeouts <socket-timeouts>`.
+ .. versionchanged:: 3.7
+ The method no longer toggles :const:`SOCK_NONBLOCK` flag on
+ :attr:`socket.type`.
+
.. method:: socket.setsockopt(level, optname, value: int)
.. method:: socket.setsockopt(level, optname, value: buffer)
diff --git a/Doc/whatsnew/3.7.rst b/Doc/whatsnew/3.7.rst
index 82f7cc07043..e5523ff7fd2 100644
--- a/Doc/whatsnew/3.7.rst
+++ b/Doc/whatsnew/3.7.rst
@@ -892,6 +892,13 @@ Changes in the Python API
recent to be more consistent with :mod:`traceback`.
(Contributed by Jesse Bakker in :issue:`32121`.)
+* On OSes that support :const:`socket.SOCK_NONBLOCK` or
+ :const:`socket.SOCK_CLOEXEC` bit flags, the
+ :attr:`socket.type <socket.socket.type>` no longer has them applied.
+ Therefore, checks like ``if sock.type == socket.SOCK_STREAM``
+ work as expected on all platforms.
+ (Contributed by Yury Selivanov in :issue:`32331`.)
+
.. _Unicode Technical Standard #18: https://unicode.org/reports/tr18/
* On Windows the default for the *close_fds* argument of
diff --git a/Lib/socket.py b/Lib/socket.py
index 1ada24d3326..2d8aee3e904 100644
--- a/Lib/socket.py
+++ b/Lib/socket.py
@@ -203,11 +203,7 @@ def accept(self):
For IP sockets, the address info is a pair (hostaddr, port).
"""
fd, addr = self._accept()
- # If our type has the SOCK_NONBLOCK flag, we shouldn't pass it onto the
- # new socket. We do not currently allow passing SOCK_NONBLOCK to
- # accept4, so the returned socket is always blocking.
- type = self.type & ~globals().get("SOCK_NONBLOCK", 0)
- sock = socket(self.family, type, self.proto, fileno=fd)
+ sock = socket(self.family, self.type, self.proto, fileno=fd)
# Issue #7995: if no default timeout is set and the listening
# socket had a (non-zero) timeout, force the new socket in blocking
# mode to override platform-specific socket flags inheritance.
diff --git a/Lib/test/test_asyncore.py b/Lib/test/test_asyncore.py
index ee0c3b371f8..694ddffd687 100644
--- a/Lib/test/test_asyncore.py
+++ b/Lib/test/test_asyncore.py
@@ -726,14 +726,10 @@ def test_connection_attributes(self):
def test_create_socket(self):
s = asyncore.dispatcher()
s.create_socket(self.family)
+ self.assertEqual(s.socket.type, socket.SOCK_STREAM)
self.assertEqual(s.socket.family, self.family)
- SOCK_NONBLOCK = getattr(socket, 'SOCK_NONBLOCK', 0)
- sock_type = socket.SOCK_STREAM | SOCK_NONBLOCK
- if hasattr(socket, 'SOCK_CLOEXEC'):
- self.assertIn(s.socket.type,
- (sock_type | socket.SOCK_CLOEXEC, sock_type))
- else:
- self.assertEqual(s.socket.type, sock_type)
+ self.assertEqual(s.socket.gettimeout(), 0)
+ self.assertFalse(s.socket.get_inheritable())
def test_bind(self):
if HAS_UNIX_SOCKETS and self.family == socket.AF_UNIX:
diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py
index 5b4c5f9f8ca..43688eacf06 100644
--- a/Lib/test/test_socket.py
+++ b/Lib/test/test_socket.py
@@ -1577,6 +1577,22 @@ def test_str_for_enums(self):
self.assertEqual(str(s.family), 'AddressFamily.AF_INET')
self.assertEqual(str(s.type), 'SocketKind.SOCK_STREAM')
+ def test_socket_consistent_sock_type(self):
+ SOCK_NONBLOCK = getattr(socket, 'SOCK_NONBLOCK', 0)
+ SOCK_CLOEXEC = getattr(socket, 'SOCK_CLOEXEC', 0)
+ sock_type = socket.SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC
+
+ with socket.socket(socket.AF_INET, sock_type) as s:
+ self.assertEqual(s.type, socket.SOCK_STREAM)
+ s.settimeout(1)
+ self.assertEqual(s.type, socket.SOCK_STREAM)
+ s.settimeout(0)
+ self.assertEqual(s.type, socket.SOCK_STREAM)
+ s.setblocking(True)
+ self.assertEqual(s.type, socket.SOCK_STREAM)
+ s.setblocking(False)
+ self.assertEqual(s.type, socket.SOCK_STREAM)
+
@unittest.skipIf(os.name == 'nt', 'Will not work on Windows')
def test_uknown_socket_family_repr(self):
# Test that when created with a family that's not one of the known
@@ -1589,9 +1605,18 @@ def test_uknown_socket_family_repr(self):
# On Windows this trick won't work, so the test is skipped.
fd, path = tempfile.mkstemp()
self.addCleanup(os.unlink, path)
- with socket.socket(family=42424, type=13331, fileno=fd) as s:
- self.assertEqual(s.family, 42424)
- self.assertEqual(s.type, 13331)
+ unknown_family = max(socket.AddressFamily.__members__.values()) + 1
+
+ unknown_type = max(
+ kind
+ for name, kind in socket.SocketKind.__members__.items()
+ if name not in {'SOCK_NONBLOCK', 'SOCK_CLOEXEC'}
+ ) + 1
+
+ with socket.socket(
+ family=unknown_family, type=unknown_type, fileno=fd) as s:
+ self.assertEqual(s.family, unknown_family)
+ self.assertEqual(s.type, unknown_type)
@unittest.skipUnless(hasattr(os, 'sendfile'), 'test needs os.sendfile()')
def test__sendfile_use_sendfile(self):
@@ -5084,7 +5109,7 @@ class InheritanceTest(unittest.TestCase):
def test_SOCK_CLOEXEC(self):
with socket.socket(socket.AF_INET,
socket.SOCK_STREAM | socket.SOCK_CLOEXEC) as s:
- self.assertTrue(s.type & socket.SOCK_CLOEXEC)
+ self.assertEqual(s.type, socket.SOCK_STREAM)
self.assertFalse(s.get_inheritable())
def test_default_inheritable(self):
@@ -5149,11 +5174,15 @@ def test_socketpair(self):
class NonblockConstantTest(unittest.TestCase):
def checkNonblock(self, s, nonblock=True, timeout=0.0):
if nonblock:
- self.assertTrue(s.type & socket.SOCK_NONBLOCK)
+ self.assertEqual(s.type, socket.SOCK_STREAM)
self.assertEqual(s.gettimeout(), timeout)
+ self.assertTrue(
+ fcntl.fcntl(s, fcntl.F_GETFL, os.O_NONBLOCK) & os.O_NONBLOCK)
else:
- self.assertFalse(s.type & socket.SOCK_NONBLOCK)
+ self.assertEqual(s.type, socket.SOCK_STREAM)
self.assertEqual(s.gettimeout(), None)
+ self.assertFalse(
+ fcntl.fcntl(s, fcntl.F_GETFL, os.O_NONBLOCK) & os.O_NONBLOCK)
@support.requires_linux_version(2, 6, 28)
def test_SOCK_NONBLOCK(self):
diff --git a/Misc/NEWS.d/next/Library/2017-12-15-23-48-43.bpo-32331.fIg1Uc.rst b/Misc/NEWS.d/next/Library/2017-12-15-23-48-43.bpo-32331.fIg1Uc.rst
new file mode 100644
index 00000000000..53a262cf220
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2017-12-15-23-48-43.bpo-32331.fIg1Uc.rst
@@ -0,0 +1,4 @@
+Fix socket.settimeout() and socket.setblocking() to keep socket.type
+as is. Fix socket.socket() constructor to reset any bit flags applied to
+socket's type. This change only affects OSes that have SOCK_NONBLOCK
+and/or SOCK_CLOEXEC.
diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c
index 91c879f9d63..d52d9db743a 100644
--- a/Modules/socketmodule.c
+++ b/Modules/socketmodule.c
@@ -582,12 +582,6 @@ internal_setblocking(PySocketSockObject *s, int block)
&& !((defined(HAVE_SYS_IOCTL_H) && defined(FIONBIO)))
int delay_flag, new_delay_flag;
#endif
-#ifdef SOCK_NONBLOCK
- if (block)
- s->sock_type &= (~SOCK_NONBLOCK);
- else
- s->sock_type |= SOCK_NONBLOCK;
-#endif
Py_BEGIN_ALLOW_THREADS
#ifndef MS_WINDOWS
@@ -876,7 +870,22 @@ init_sockobject(PySocketSockObject *s,
{
s->sock_fd = fd;
s->sock_family = family;
+
s->sock_type = type;
+
+ /* It's possible to pass SOCK_NONBLOCK and SOCK_CLOEXEC bit flags
+ on some OSes as part of socket.type. We want to reset them here,
+ to make socket.type be set to the same value on all platforms.
+ Otherwise, simple code like 'if sock.type == SOCK_STREAM' is
+ not portable.
+ */
+#ifdef SOCK_NONBLOCK
+ s->sock_type = s->sock_type & ~SOCK_NONBLOCK;
+#endif
+#ifdef SOCK_CLOEXEC
+ s->sock_type = s->sock_type & ~SOCK_CLOEXEC;
+#endif
+
s->sock_proto = proto;
s->errorhandler = &set_error;
More information about the Python-checkins
mailing list