Mailman 3 November 2018 - Python-checkins

bpo-33015: Fix UB in pthread PyThread_start_new_thread (GH-6008)
by Victor Stinner 30 Nov '18

30 Nov '18

https://github.com/python/cpython/commit/9eea6eaf23067880f4af3a130e3f67c981… commit: 9eea6eaf23067880f4af3a130e3f67c9812e2f30 branch: master author: Siddhesh Poyarekar <siddhesh.poyarekar(a)gmail.com> committer: Victor Stinner <vstinner(a)redhat.com> date: 2018-11-30T16:14:25+01:00 summary: bpo-33015: Fix UB in pthread PyThread_start_new_thread (GH-6008) Fix an undefined behaviour in the pthread implementation of PyThread_start_new_thread(): add a function wrapper to always return NULL. Add pythread_callback struct and pythread_wrapper() to thread_pthread.h. files: A Misc/NEWS.d/next/Build/2018-08-24-09-48-25.bpo-33015.s21y74.rst M Python/thread_pthread.h diff --git a/Misc/NEWS.d/next/Build/2018-08-24-09-48-25.bpo-33015.s21y74.rst b/Misc/NEWS.d/next/Build/2018-08-24-09-48-25.bpo-33015.s21y74.rst new file mode 100644 index 000000000000..8c5a0c91a46b --- /dev/null +++ b/Misc/NEWS.d/next/Build/2018-08-24-09-48-25.bpo-33015.s21y74.rst @@ -0,0 +1,3 @@ +Fix an undefined behaviour in the pthread implementation of +:c:func:`PyThread_start_new_thread`: add a function wrapper to always return +``NULL``. diff --git a/Python/thread_pthread.h b/Python/thread_pthread.h index 6da8b3a44734..09e53a14aa47 100644 --- a/Python/thread_pthread.h +++ b/Python/thread_pthread.h @@ -152,6 +152,28 @@ PyThread__init_thread(void) * Thread support. */ +/* bpo-33015: pythread_callback struct and pythread_wrapper() cast + "void func(void *)" to "void* func(void *)": always return NULL. + + PyThread_start_new_thread() uses "void func(void *)" type, whereas + pthread_create() requires a void* return value. */ +typedef struct { + void (*func) (void *); + void *arg; +} pythread_callback; + +static void * +pythread_wrapper(void *arg) +{ + /* copy func and func_arg and free the temporary structure */ + pythread_callback *callback = arg; + void (*func)(void *) = callback->func; + void *func_arg = callback->arg; + PyMem_RawFree(arg); + + func(func_arg); + return NULL; +} unsigned long PyThread_start_new_thread(void (*func)(void *), void *arg) @@ -188,21 +210,31 @@ PyThread_start_new_thread(void (*func)(void *), void *arg) pthread_attr_setscope(&attrs, PTHREAD_SCOPE_SYSTEM); #endif + pythread_callback *callback = PyMem_RawMalloc(sizeof(pythread_callback)); + + if (callback == NULL) { + return PYTHREAD_INVALID_THREAD_ID; + } + + callback->func = func; + callback->arg = arg; + status = pthread_create(&th, #if defined(THREAD_STACK_SIZE) || defined(PTHREAD_SYSTEM_SCHED_SUPPORTED) &attrs, #else (pthread_attr_t*)NULL, #endif - (void* (*)(void *))func, - (void *)arg - ); + pythread_wrapper, callback); #if defined(THREAD_STACK_SIZE) || defined(PTHREAD_SYSTEM_SCHED_SUPPORTED) pthread_attr_destroy(&attrs); #endif - if (status != 0) + + if (status != 0) { + PyMem_RawFree(callback); return PYTHREAD_INVALID_THREAD_ID; + } pthread_detach(th);

1 0

Fix compiler warning in call_readline() (GH-10820)
by Victor Stinner 30 Nov '18

30 Nov '18

https://github.com/python/cpython/commit/1600f60414e620c4298c15dac803427d8f… commit: 1600f60414e620c4298c15dac803427d8f0a977c branch: master author: Victor Stinner <vstinner(a)redhat.com> committer: GitHub <noreply(a)github.com> date: 2018-11-30T15:03:53+01:00 summary: Fix compiler warning in call_readline() (GH-10820) Replace strncpy() with memcpy() in call_readline() to fix the following warning, the NUL byte is written manually just after: Modules/readline.c: In function ‘call_readline’: Modules/readline.c:1303:9: warning: ‘strncpy’ output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation] strncpy(p, q, n); ^~~~~~~~~~~~~~~~ Modules/readline.c:1279:9: note: length computed here n = strlen(p); ^~~~~~~~~ files: M Modules/readline.c diff --git a/Modules/readline.c b/Modules/readline.c index 7756e6b2bc75..7f366aabfb4f 100644 --- a/Modules/readline.c +++ b/Modules/readline.c @@ -1240,7 +1240,7 @@ static char * call_readline(FILE *sys_stdin, FILE *sys_stdout, const char *prompt) { size_t n; - char *p, *q; + char *p; int signal; #ifdef SAVE_LOCALE @@ -1297,10 +1297,10 @@ call_readline(FILE *sys_stdin, FILE *sys_stdout, const char *prompt) } /* Copy the malloc'ed buffer into a PyMem_Malloc'ed one and release the original. */ - q = p; + char *q = p; p = PyMem_RawMalloc(n+2); if (p != NULL) { - strncpy(p, q, n); + memcpy(p, q, n); p[n] = '\n'; p[n+1] = '\0'; }

1 0

Fix signature of xml.dom.minidom.Document.toprettyxml(). (GH-10814)
by Serhiy Storchaka 30 Nov '18

30 Nov '18

https://github.com/python/cpython/commit/b7c2182604d5796b5af4c837991aa0b8c8… commit: b7c2182604d5796b5af4c837991aa0b8c8a2d41f branch: master author: E Kawashima <e-kwsm(a)users.noreply.github.com> committer: Serhiy Storchaka <storchaka(a)gmail.com> date: 2018-11-30T16:03:00+02:00 summary: Fix signature of xml.dom.minidom.Document.toprettyxml(). (GH-10814) files: M Doc/library/xml.dom.minidom.rst diff --git a/Doc/library/xml.dom.minidom.rst b/Doc/library/xml.dom.minidom.rst index a1f334d4178e..42340802f193 100644 --- a/Doc/library/xml.dom.minidom.rst +++ b/Doc/library/xml.dom.minidom.rst @@ -163,7 +163,7 @@ module documentation. This section lists the differences between the API and The :meth:`toxml` method now preserves the attribute order specified by the user. -.. method:: Node.toprettyxml(indent="", newl="", encoding="") +.. method:: Node.toprettyxml(indent="\t", newl="\n", encoding=None) Return a pretty-printed version of the document. *indent* specifies the indentation string and defaults to a tabulator; *newl* specifies the string

1 0

bpo-35347: Cleanup test_socket.NonBlockingTCPTests (GH-10818)
by Victor Stinner 30 Nov '18

30 Nov '18

https://github.com/python/cpython/commit/304315d251dbb4e85dd86056ba1925f25e… commit: 304315d251dbb4e85dd86056ba1925f25e646ca1 branch: master author: Victor Stinner <vstinner(a)redhat.com> committer: GitHub <noreply(a)github.com> date: 2018-11-30T13:22:44+01:00 summary: bpo-35347: Cleanup test_socket.NonBlockingTCPTests (GH-10818) * Replace testInheritFlags() with two tests: testInheritFlagsBlocking() and testInheritFlagsTimeout() to test different default socket timeout. Moreover, the test now checks sock.gettimeout() rather than a functional test on recv(). * Replace time.time() with time.monotonic() * Add socket_setdefaulttimeout() context manager to restore the default timeout when the test completes. * Remove testConnect(): accept() wasn't blocking and testAccept() already tests non-blocking accept(). * Remove accept() functional test from testInitNonBlocking(): already tested by testAccept() * Rewrite testSetBlocking() with a new assert_sock_timeout() method * Use addCleanup() and context manager to close sockets * Replace assertTrue(x < y) with assertLess(x, y) files: M Lib/test/test_socket.py diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index ececebfd1b12..a2c047daa3a4 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -34,7 +34,8 @@ fcntl = None HOST = support.HOST -MSG = 'Michael Gilfix was here\u1234\r\n'.encode('utf-8') ## test unicode string and carriage return +# test unicode string and carriage return +MSG = 'Michael Gilfix was here\u1234\r\n'.encode('utf-8') MAIN_TIMEOUT = 60.0 VSOCKPORT = 1234 @@ -111,9 +112,14 @@ def _have_socket_vsock(): return ret -def _is_fd_in_blocking_mode(sock): - return not bool( - fcntl.fcntl(sock, fcntl.F_GETFL, os.O_NONBLOCK) & os.O_NONBLOCK) +(a)contextlib.contextmanager +def socket_setdefaulttimeout(timeout): + old_timeout = socket.getdefaulttimeout() + try: + socket.setdefaulttimeout(timeout) + yield + finally: + socket.setdefaulttimeout(old_timeout) HAVE_SOCKET_CAN = _have_socket_can() @@ -1069,18 +1075,16 @@ def testDefaultTimeout(self): s.close() # Set the default timeout to 10, and see if it propagates - socket.setdefaulttimeout(10) - self.assertEqual(socket.getdefaulttimeout(), 10) - s = socket.socket() - self.assertEqual(s.gettimeout(), 10) - s.close() + with socket_setdefaulttimeout(10): + self.assertEqual(socket.getdefaulttimeout(), 10) + with socket.socket() as sock: + self.assertEqual(sock.gettimeout(), 10) - # Reset the default timeout to None, and see if it propagates - socket.setdefaulttimeout(None) - self.assertEqual(socket.getdefaulttimeout(), None) - s = socket.socket() - self.assertEqual(s.gettimeout(), None) - s.close() + # Reset the default timeout to None, and see if it propagates + socket.setdefaulttimeout(None) + self.assertEqual(socket.getdefaulttimeout(), None) + with socket.socket() as sock: + self.assertEqual(sock.gettimeout(), None) # Check that setting it to an invalid value raises ValueError self.assertRaises(ValueError, socket.setdefaulttimeout, -1) @@ -4218,55 +4222,42 @@ def __init__(self, methodName='runTest'): self.event = threading.Event() ThreadedTCPSocketTest.__init__(self, methodName=methodName) + def assert_sock_timeout(self, sock, timeout): + self.assertEqual(self.serv.gettimeout(), timeout) + + blocking = (timeout != 0.0) + self.assertEqual(sock.getblocking(), blocking) + + if fcntl is not None: + # When a Python socket has a non-zero timeout, it's switched + # internally to a non-blocking mode. Later, sock.sendall(), + # sock.recv(), and other socket operations use a select() call and + # handle EWOULDBLOCK/EGAIN on all socket operations. That's how + # timeouts are enforced. + fd_blocking = (timeout is None) + + flag = fcntl.fcntl(sock, fcntl.F_GETFL, os.O_NONBLOCK) + self.assertEqual(not bool(flag & os.O_NONBLOCK), fd_blocking) + def testSetBlocking(self): - # Testing whether set blocking works + # Test setblocking() and settimeout() methods self.serv.setblocking(True) - self.assertIsNone(self.serv.gettimeout()) - self.assertTrue(self.serv.getblocking()) - if fcntl: - self.assertTrue(_is_fd_in_blocking_mode(self.serv)) + self.assert_sock_timeout(self.serv, None) self.serv.setblocking(False) - self.assertEqual(self.serv.gettimeout(), 0.0) - self.assertFalse(self.serv.getblocking()) - if fcntl: - self.assertFalse(_is_fd_in_blocking_mode(self.serv)) + self.assert_sock_timeout(self.serv, 0.0) self.serv.settimeout(None) - self.assertTrue(self.serv.getblocking()) - if fcntl: - self.assertTrue(_is_fd_in_blocking_mode(self.serv)) + self.assert_sock_timeout(self.serv, None) self.serv.settimeout(0) - self.assertFalse(self.serv.getblocking()) - self.assertEqual(self.serv.gettimeout(), 0) - if fcntl: - self.assertFalse(_is_fd_in_blocking_mode(self.serv)) + self.assert_sock_timeout(self.serv, 0) self.serv.settimeout(10) - self.assertTrue(self.serv.getblocking()) - self.assertEqual(self.serv.gettimeout(), 10) - if fcntl: - # When a Python socket has a non-zero timeout, it's - # switched internally to a non-blocking mode. - # Later, sock.sendall(), sock.recv(), and other socket - # operations use a `select()` call and handle EWOULDBLOCK/EGAIN - # on all socket operations. That's how timeouts are - # enforced. - self.assertFalse(_is_fd_in_blocking_mode(self.serv)) + self.assert_sock_timeout(self.serv, 10) self.serv.settimeout(0) - self.assertFalse(self.serv.getblocking()) - if fcntl: - self.assertFalse(_is_fd_in_blocking_mode(self.serv)) - - start = time.time() - try: - self.serv.accept() - except OSError: - pass - end = time.time() - self.assertTrue((end - start) < 1.0, "Error setting non-blocking mode.") + self.assert_sock_timeout(self.serv, 0) def _testSetBlocking(self): pass @@ -4277,8 +4268,10 @@ def testSetBlocking_overflow(self): import _testcapi if _testcapi.UINT_MAX >= _testcapi.ULONG_MAX: self.skipTest('needs UINT_MAX < ULONG_MAX') + self.serv.setblocking(False) self.assertEqual(self.serv.gettimeout(), 0.0) + self.serv.setblocking(_testcapi.UINT_MAX + 1) self.assertIsNone(self.serv.gettimeout()) @@ -4288,50 +4281,51 @@ def testSetBlocking_overflow(self): 'test needs socket.SOCK_NONBLOCK') @support.requires_linux_version(2, 6, 28) def testInitNonBlocking(self): - # reinit server socket + # create a socket with SOCK_NONBLOCK self.serv.close() - self.serv = socket.socket(socket.AF_INET, socket.SOCK_STREAM | - socket.SOCK_NONBLOCK) - self.assertFalse(self.serv.getblocking()) - self.assertEqual(self.serv.gettimeout(), 0) - self.port = support.bind_port(self.serv) - self.serv.listen() - # actual testing - start = time.time() - try: - self.serv.accept() - except OSError: - pass - end = time.time() - self.assertTrue((end - start) < 1.0, "Error creating with non-blocking mode.") + self.serv = socket.socket(socket.AF_INET, + socket.SOCK_STREAM | socket.SOCK_NONBLOCK) + self.assert_sock_timeout(self.serv, 0) def _testInitNonBlocking(self): pass - def testInheritFlags(self): - # Issue #7995: when calling accept() on a listening socket with a - # timeout, the resulting socket should not be non-blocking. - self.serv.settimeout(10) - try: + def testInheritFlagsBlocking(self): + # bpo-7995: accept() on a listening socket with a timeout and the + # default timeout is None, the resulting socket must be blocking. + with socket_setdefaulttimeout(None): + self.serv.settimeout(10) conn, addr = self.serv.accept() - message = conn.recv(len(MSG)) - finally: - conn.close() - self.serv.settimeout(None) + self.addCleanup(conn.close) + self.assertIsNone(conn.gettimeout()) - def _testInheritFlags(self): - time.sleep(0.1) + def _testInheritFlagsBlocking(self): + self.cli.connect((HOST, self.port)) + + def testInheritFlagsTimeout(self): + # bpo-7995: accept() on a listening socket with a timeout and the + # default timeout is None, the resulting socket must inherit + # the default timeout. + default_timeout = 20.0 + with socket_setdefaulttimeout(default_timeout): + self.serv.settimeout(10) + conn, addr = self.serv.accept() + self.addCleanup(conn.close) + self.assertEqual(conn.gettimeout(), default_timeout) + + def _testInheritFlagsTimeout(self): self.cli.connect((HOST, self.port)) - time.sleep(0.5) - self.cli.send(MSG) def testAccept(self): # Testing non-blocking accept self.serv.setblocking(0) # connect() didn't start: non-blocking accept() fails + start_time = time.monotonic() with self.assertRaises(BlockingIOError): conn, addr = self.serv.accept() + dt = time.monotonic() - start_time + self.assertLess(dt, 1.0) self.event.set() @@ -4351,15 +4345,6 @@ def _testAccept(self): self.cli.connect((HOST, self.port)) - def testConnect(self): - # Testing non-blocking connect - conn, addr = self.serv.accept() - conn.close() - - def _testConnect(self): - self.cli.settimeout(10) - self.cli.connect((HOST, self.port)) - def testRecv(self): # Testing non-blocking recv conn, addr = self.serv.accept()

1 0

bpo-35347: Fix test_socket.NonBlockingTCPTests (GH-10791) (GH-10817)
by Victor Stinner 30 Nov '18

30 Nov '18

https://github.com/python/cpython/commit/dab59fa56054d6c0f75ae7013337f7baaa… commit: dab59fa56054d6c0f75ae7013337f7baaa248076 branch: 2.7 author: Victor Stinner <vstinner(a)redhat.com> committer: GitHub <noreply(a)github.com> date: 2018-11-30T13:02:41+01:00 summary: bpo-35347: Fix test_socket.NonBlockingTCPTests (GH-10791) (GH-10817) testAccept() and testRecv() of test_socket.NonBlockingTCPTests have a race condition: time.sleep() is used as a weak synchronization primitive and the tests fail randomly on slow buildbots. Use a reliable threading.Event to fix these tests. Other changes: * Replace send() with sendall() * Add a timeout to select() in testAccept() and testRecv() * Use addCleanup() to close sockets * Use assertRaises() (cherry picked from commit ebd5d6d6e6e4e751ba9c7534004aadfc27ba9265) files: M Lib/test/test_socket.py diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index 14607115928b..988e12a813d5 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -21,6 +21,9 @@ _socket = None +MAIN_TIMEOUT = 60.0 + + def try_address(host, port=0, family=socket.AF_INET): """Try to bind a socket on the given host:port and return True if that has been possible.""" @@ -947,6 +950,7 @@ def _testSend(self): class NonBlockingTCPTests(ThreadedTCPSocketTest): def __init__(self, methodName='runTest'): + self.event = threading.Event() ThreadedTCPSocketTest.__init__(self, methodName=methodName) def testSetBlocking(self): @@ -982,21 +986,27 @@ def testSetBlocking_overflow(self): def testAccept(self): # Testing non-blocking accept self.serv.setblocking(0) - try: - conn, addr = self.serv.accept() - except socket.error: - pass - else: - self.fail("Error trying to do non-blocking accept.") - read, write, err = select.select([self.serv], [], []) - if self.serv in read: + + # connect() didn't start: non-blocking accept() fails + with self.assertRaises(socket.error): conn, addr = self.serv.accept() - conn.close() - else: + + self.event.set() + + read, write, err = select.select([self.serv], [], [], MAIN_TIMEOUT) + if self.serv not in read: self.fail("Error trying to do accept after select.") + # connect() completed: non-blocking accept() doesn't block + conn, addr = self.serv.accept() + self.addCleanup(conn.close) + self.assertIsNone(conn.gettimeout()) + def _testAccept(self): - time.sleep(0.1) + # don't connect before event is set to check + # that non-blocking accept() raises socket.error + self.event.wait() + self.cli.connect((HOST, self.port)) def testConnect(self): @@ -1011,25 +1021,32 @@ def _testConnect(self): def testRecv(self): # Testing non-blocking recv conn, addr = self.serv.accept() + self.addCleanup(conn.close) conn.setblocking(0) - try: - msg = conn.recv(len(MSG)) - except socket.error: - pass - else: - self.fail("Error trying to do non-blocking recv.") - read, write, err = select.select([conn], [], []) - if conn in read: + + # the server didn't send data yet: non-blocking recv() fails + with self.assertRaises(socket.error): msg = conn.recv(len(MSG)) - conn.close() - self.assertEqual(msg, MSG) - else: + + self.event.set() + + read, write, err = select.select([conn], [], [], MAIN_TIMEOUT) + if conn not in read: self.fail("Error during select call to non-blocking socket.") + # the server sent data yet: non-blocking recv() doesn't block + msg = conn.recv(len(MSG)) + self.assertEqual(msg, MSG) + def _testRecv(self): self.cli.connect((HOST, self.port)) - time.sleep(0.1) - self.cli.send(MSG) + + # don't send anything before event is set to check + # that non-blocking recv() raises socket.error + self.event.wait() + + # send data: recv() will no longer block + self.cli.sendall(MSG) @unittest.skipUnless(thread, 'Threading required for this test.') class FileObjectClassTestCase(SocketConnectedTest):

1 0

bpo-35347: Fix test_socket.NonBlockingTCPTests (GH-10791)
by Miss Islington (bot) 30 Nov '18

30 Nov '18

https://github.com/python/cpython/commit/af7e81f71858519de8d2bafcb38fce8cca… commit: af7e81f71858519de8d2bafcb38fce8cca86aa0a branch: 3.6 author: Miss Islington (bot) <31488909+miss-islington(a)users.noreply.github.com> committer: GitHub <noreply(a)github.com> date: 2018-11-30T03:51:42-08:00 summary: bpo-35347: Fix test_socket.NonBlockingTCPTests (GH-10791) testAccept() and testRecv() of test_socket.NonBlockingTCPTests have a race condition: time.sleep() is used as a weak synchronization primitive and the tests fail randomly on slow buildbots. Use a reliable threading.Event to fix these tests. Other changes: * Replace send() with sendall() * Expect specific BlockingIOError rather than generic OSError * Add a timeout to select() in testAccept() and testRecv() * Use addCleanup() to close sockets * Use assertRaises() (cherry picked from commit ebd5d6d6e6e4e751ba9c7534004aadfc27ba9265) Co-authored-by: Victor Stinner <vstinner(a)redhat.com> files: M Lib/test/test_socket.py diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index fbbc9f9abfb0..6b7afba49d9e 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -32,6 +32,7 @@ HOST = support.HOST MSG = 'Michael Gilfix was here\u1234\r\n'.encode('utf-8') ## test unicode string and carriage return +MAIN_TIMEOUT = 60.0 try: import _thread as thread @@ -3870,6 +3871,7 @@ def _testSend(self): class NonBlockingTCPTests(ThreadedTCPSocketTest): def __init__(self, methodName='runTest'): + self.event = threading.Event() ThreadedTCPSocketTest.__init__(self, methodName=methodName) def testSetBlocking(self): @@ -3944,22 +3946,27 @@ def _testInheritFlags(self): def testAccept(self): # Testing non-blocking accept self.serv.setblocking(0) - try: - conn, addr = self.serv.accept() - except OSError: - pass - else: - self.fail("Error trying to do non-blocking accept.") - read, write, err = select.select([self.serv], [], []) - if self.serv in read: + + # connect() didn't start: non-blocking accept() fails + with self.assertRaises(BlockingIOError): conn, addr = self.serv.accept() - self.assertIsNone(conn.gettimeout()) - conn.close() - else: + + self.event.set() + + read, write, err = select.select([self.serv], [], [], MAIN_TIMEOUT) + if self.serv not in read: self.fail("Error trying to do accept after select.") + # connect() completed: non-blocking accept() doesn't block + conn, addr = self.serv.accept() + self.addCleanup(conn.close) + self.assertIsNone(conn.gettimeout()) + def _testAccept(self): - time.sleep(0.1) + # don't connect before event is set to check + # that non-blocking accept() raises BlockingIOError + self.event.wait() + self.cli.connect((HOST, self.port)) def testConnect(self): @@ -3974,25 +3981,32 @@ def _testConnect(self): def testRecv(self): # Testing non-blocking recv conn, addr = self.serv.accept() + self.addCleanup(conn.close) conn.setblocking(0) - try: - msg = conn.recv(len(MSG)) - except OSError: - pass - else: - self.fail("Error trying to do non-blocking recv.") - read, write, err = select.select([conn], [], []) - if conn in read: + + # the server didn't send data yet: non-blocking recv() fails + with self.assertRaises(BlockingIOError): msg = conn.recv(len(MSG)) - conn.close() - self.assertEqual(msg, MSG) - else: + + self.event.set() + + read, write, err = select.select([conn], [], [], MAIN_TIMEOUT) + if conn not in read: self.fail("Error during select call to non-blocking socket.") + # the server sent data yet: non-blocking recv() doesn't block + msg = conn.recv(len(MSG)) + self.assertEqual(msg, MSG) + def _testRecv(self): self.cli.connect((HOST, self.port)) - time.sleep(0.1) - self.cli.send(MSG) + + # don't send anything before event is set to check + # that non-blocking recv() raises BlockingIOError + self.event.wait() + + # send data: recv() will no longer block + self.cli.sendall(MSG) @unittest.skipUnless(thread, 'Threading required for this test.') class FileObjectClassTestCase(SocketConnectedTest):

1 0

bpo-35347: Fix test_socket.NonBlockingTCPTests (GH-10791)
by Miss Islington (bot) 30 Nov '18

30 Nov '18

https://github.com/python/cpython/commit/365f21c2d3756a6768c5b0d479aaf5c2a5… commit: 365f21c2d3756a6768c5b0d479aaf5c2a568b80b branch: 3.7 author: Miss Islington (bot) <31488909+miss-islington(a)users.noreply.github.com> committer: GitHub <noreply(a)github.com> date: 2018-11-30T03:48:17-08:00 summary: bpo-35347: Fix test_socket.NonBlockingTCPTests (GH-10791) testAccept() and testRecv() of test_socket.NonBlockingTCPTests have a race condition: time.sleep() is used as a weak synchronization primitive and the tests fail randomly on slow buildbots. Use a reliable threading.Event to fix these tests. Other changes: * Replace send() with sendall() * Expect specific BlockingIOError rather than generic OSError * Add a timeout to select() in testAccept() and testRecv() * Use addCleanup() to close sockets * Use assertRaises() (cherry picked from commit ebd5d6d6e6e4e751ba9c7534004aadfc27ba9265) Co-authored-by: Victor Stinner <vstinner(a)redhat.com> files: M Lib/test/test_socket.py diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index 5f9891de4936..db2160eaf583 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -35,6 +35,7 @@ HOST = support.HOST MSG = 'Michael Gilfix was here\u1234\r\n'.encode('utf-8') ## test unicode string and carriage return +MAIN_TIMEOUT = 60.0 VSOCKPORT = 1234 @@ -4174,6 +4175,7 @@ def _testSend(self): class NonBlockingTCPTests(ThreadedTCPSocketTest): def __init__(self, methodName='runTest'): + self.event = threading.Event() ThreadedTCPSocketTest.__init__(self, methodName=methodName) def testSetBlocking(self): @@ -4286,22 +4288,27 @@ def _testInheritFlags(self): def testAccept(self): # Testing non-blocking accept self.serv.setblocking(0) - try: - conn, addr = self.serv.accept() - except OSError: - pass - else: - self.fail("Error trying to do non-blocking accept.") - read, write, err = select.select([self.serv], [], []) - if self.serv in read: + + # connect() didn't start: non-blocking accept() fails + with self.assertRaises(BlockingIOError): conn, addr = self.serv.accept() - self.assertIsNone(conn.gettimeout()) - conn.close() - else: + + self.event.set() + + read, write, err = select.select([self.serv], [], [], MAIN_TIMEOUT) + if self.serv not in read: self.fail("Error trying to do accept after select.") + # connect() completed: non-blocking accept() doesn't block + conn, addr = self.serv.accept() + self.addCleanup(conn.close) + self.assertIsNone(conn.gettimeout()) + def _testAccept(self): - time.sleep(0.1) + # don't connect before event is set to check + # that non-blocking accept() raises BlockingIOError + self.event.wait() + self.cli.connect((HOST, self.port)) def testConnect(self): @@ -4316,25 +4323,32 @@ def _testConnect(self): def testRecv(self): # Testing non-blocking recv conn, addr = self.serv.accept() + self.addCleanup(conn.close) conn.setblocking(0) - try: - msg = conn.recv(len(MSG)) - except OSError: - pass - else: - self.fail("Error trying to do non-blocking recv.") - read, write, err = select.select([conn], [], []) - if conn in read: + + # the server didn't send data yet: non-blocking recv() fails + with self.assertRaises(BlockingIOError): msg = conn.recv(len(MSG)) - conn.close() - self.assertEqual(msg, MSG) - else: + + self.event.set() + + read, write, err = select.select([conn], [], [], MAIN_TIMEOUT) + if conn not in read: self.fail("Error during select call to non-blocking socket.") + # the server sent data yet: non-blocking recv() doesn't block + msg = conn.recv(len(MSG)) + self.assertEqual(msg, MSG) + def _testRecv(self): self.cli.connect((HOST, self.port)) - time.sleep(0.1) - self.cli.send(MSG) + + # don't send anything before event is set to check + # that non-blocking recv() raises BlockingIOError + self.event.wait() + + # send data: recv() will no longer block + self.cli.sendall(MSG) class FileObjectClassTestCase(SocketConnectedTest):

1 0

bpo-35347: Fix test_socket.NonBlockingTCPTests (GH-10791)
by Victor Stinner 30 Nov '18

30 Nov '18

https://github.com/python/cpython/commit/ebd5d6d6e6e4e751ba9c7534004aadfc27… commit: ebd5d6d6e6e4e751ba9c7534004aadfc27ba9265 branch: master author: Victor Stinner <vstinner(a)redhat.com> committer: GitHub <noreply(a)github.com> date: 2018-11-30T12:29:25+01:00 summary: bpo-35347: Fix test_socket.NonBlockingTCPTests (GH-10791) testAccept() and testRecv() of test_socket.NonBlockingTCPTests have a race condition: time.sleep() is used as a weak synchronization primitive and the tests fail randomly on slow buildbots. Use a reliable threading.Event to fix these tests. Other changes: * Replace send() with sendall() * Expect specific BlockingIOError rather than generic OSError * Add a timeout to select() in testAccept() and testRecv() * Use addCleanup() to close sockets * Use assertRaises() files: M Lib/test/test_socket.py diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index 663a018dcfda..ececebfd1b12 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -35,6 +35,7 @@ HOST = support.HOST MSG = 'Michael Gilfix was here\u1234\r\n'.encode('utf-8') ## test unicode string and carriage return +MAIN_TIMEOUT = 60.0 VSOCKPORT = 1234 @@ -4214,6 +4215,7 @@ def _testSend(self): class NonBlockingTCPTests(ThreadedTCPSocketTest): def __init__(self, methodName='runTest'): + self.event = threading.Event() ThreadedTCPSocketTest.__init__(self, methodName=methodName) def testSetBlocking(self): @@ -4326,22 +4328,27 @@ def _testInheritFlags(self): def testAccept(self): # Testing non-blocking accept self.serv.setblocking(0) - try: - conn, addr = self.serv.accept() - except OSError: - pass - else: - self.fail("Error trying to do non-blocking accept.") - read, write, err = select.select([self.serv], [], []) - if self.serv in read: + + # connect() didn't start: non-blocking accept() fails + with self.assertRaises(BlockingIOError): conn, addr = self.serv.accept() - self.assertIsNone(conn.gettimeout()) - conn.close() - else: + + self.event.set() + + read, write, err = select.select([self.serv], [], [], MAIN_TIMEOUT) + if self.serv not in read: self.fail("Error trying to do accept after select.") + # connect() completed: non-blocking accept() doesn't block + conn, addr = self.serv.accept() + self.addCleanup(conn.close) + self.assertIsNone(conn.gettimeout()) + def _testAccept(self): - time.sleep(0.1) + # don't connect before event is set to check + # that non-blocking accept() raises BlockingIOError + self.event.wait() + self.cli.connect((HOST, self.port)) def testConnect(self): @@ -4356,25 +4363,32 @@ def _testConnect(self): def testRecv(self): # Testing non-blocking recv conn, addr = self.serv.accept() + self.addCleanup(conn.close) conn.setblocking(0) - try: - msg = conn.recv(len(MSG)) - except OSError: - pass - else: - self.fail("Error trying to do non-blocking recv.") - read, write, err = select.select([conn], [], []) - if conn in read: + + # the server didn't send data yet: non-blocking recv() fails + with self.assertRaises(BlockingIOError): msg = conn.recv(len(MSG)) - conn.close() - self.assertEqual(msg, MSG) - else: + + self.event.set() + + read, write, err = select.select([conn], [], [], MAIN_TIMEOUT) + if conn not in read: self.fail("Error during select call to non-blocking socket.") + # the server sent data yet: non-blocking recv() doesn't block + msg = conn.recv(len(MSG)) + self.assertEqual(msg, MSG) + def _testRecv(self): self.cli.connect((HOST, self.port)) - time.sleep(0.1) - self.cli.send(MSG) + + # don't send anything before event is set to check + # that non-blocking recv() raises BlockingIOError + self.event.wait() + + # send data: recv() will no longer block + self.cli.sendall(MSG) class FileObjectClassTestCase(SocketConnectedTest):

1 0

bpo-35336: Fix PYTHONCOERCECLOCALE=1 (GH-10806) (GH-10813)
by Victor Stinner 30 Nov '18

30 Nov '18

https://github.com/python/cpython/commit/df738d56fe798b3586ed71775df25bf127… commit: df738d56fe798b3586ed71775df25bf127789cf6 branch: 3.7 author: Victor Stinner <vstinner(a)redhat.com> committer: GitHub <noreply(a)github.com> date: 2018-11-30T12:19:48+01:00 summary: bpo-35336: Fix PYTHONCOERCECLOCALE=1 (GH-10806) (GH-10813) Fix PYTHONCOERCECLOCALE=1 environment variable: only coerce the C locale if the LC_CTYPE locale is "C". (cherry picked from commit 55e498058faf8c97840556f6d791c2c392732dc3) files: A Misc/NEWS.d/next/Core and Builtins/2018-11-29-23-59-52.bpo-35336.8LOz4F.rst M Lib/test/test_c_locale_coercion.py M Modules/main.c diff --git a/Lib/test/test_c_locale_coercion.py b/Lib/test/test_c_locale_coercion.py index 1db293b9c373..134f07a17ec1 100644 --- a/Lib/test/test_c_locale_coercion.py +++ b/Lib/test/test_c_locale_coercion.py @@ -1,11 +1,12 @@ # Tests the attempted automatic coercion of the C locale to a UTF-8 locale -import unittest import locale import os +import shutil +import subprocess import sys import sysconfig -import shutil +import unittest from collections import namedtuple import test.support @@ -25,6 +26,8 @@ # Set our expectation for the default locale used when none is specified EXPECT_COERCION_IN_DEFAULT_LOCALE = True +TARGET_LOCALES = ["C.UTF-8", "C.utf8", "UTF-8"] + # Apply some platform dependent overrides if sys.platform.startswith("linux"): if test.support.is_android: @@ -404,6 +407,27 @@ def test_LC_ALL_set_to_C(self): expected_warnings=[LEGACY_LOCALE_WARNING], coercion_expected=False) + def test_PYTHONCOERCECLOCALE_set_to_one(self): + # skip the test if the LC_CTYPE locale is C or coerced + old_loc = locale.setlocale(locale.LC_CTYPE, None) + self.addCleanup(locale.setlocale, locale.LC_CTYPE, old_loc) + loc = locale.setlocale(locale.LC_CTYPE, "") + if loc == "C": + self.skipTest("test requires LC_CTYPE locale different than C") + if loc in TARGET_LOCALES : + self.skipTest("coerced LC_CTYPE locale: %s" % loc) + + # bpo-35336: PYTHONCOERCECLOCALE=1 must not coerce the LC_CTYPE locale + # if it's not equal to "C" + code = 'import locale; print(locale.setlocale(locale.LC_CTYPE, None))' + env = dict(os.environ, PYTHONCOERCECLOCALE='1') + cmd = subprocess.run([sys.executable, '-c', code], + stdout=subprocess.PIPE, + env=env, + text=True) + self.assertEqual(cmd.stdout.rstrip(), loc) + + def test_main(): test.support.run_unittest( LocaleConfigurationTests, diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-11-29-23-59-52.bpo-35336.8LOz4F.rst b/Misc/NEWS.d/next/Core and Builtins/2018-11-29-23-59-52.bpo-35336.8LOz4F.rst new file mode 100644 index 000000000000..28f8f9bd4db7 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-11-29-23-59-52.bpo-35336.8LOz4F.rst @@ -0,0 +1,2 @@ +Fix PYTHONCOERCECLOCALE=1 environment variable: only coerce the C locale +if the LC_CTYPE locale is "C". diff --git a/Modules/main.c b/Modules/main.c index 6dbe6a30786b..af2c191b9b9b 100644 --- a/Modules/main.c +++ b/Modules/main.c @@ -2191,11 +2191,17 @@ pymain_read_conf(_PyMain *pymain, _PyCoreConfig *config, _PyCmdline *cmdline) static void config_init_locale(_PyCoreConfig *config) { - if (config->coerce_c_locale < 0) { + /* Test also if coerce_c_locale equals 1: PYTHONCOERCECLOCALE=1 doesn't + imply that the C locale is always coerced. It is only coerced if + if the LC_CTYPE locale is "C". */ + if (config->coerce_c_locale != 0) { /* The C locale enables the C locale coercion (PEP 538) */ if (_Py_LegacyLocaleDetected()) { config->coerce_c_locale = 1; } + else { + config->coerce_c_locale = 0; + } } #ifndef MS_WINDOWS @@ -2376,7 +2382,7 @@ _PyCoreConfig_Read(_PyCoreConfig *config) } } - if (config->utf8_mode < 0 || config->coerce_c_locale < 0) { + if (config->coerce_c_locale != 0 || config->utf8_mode < 0) { config_init_locale(config); }

1 0

bpo-35336: Fix PYTHONCOERCECLOCALE=1 (GH-10806)
by Victor Stinner 30 Nov '18

30 Nov '18

https://github.com/python/cpython/commit/55e498058faf8c97840556f6d791c2c392… commit: 55e498058faf8c97840556f6d791c2c392732dc3 branch: master author: Victor Stinner <vstinner(a)redhat.com> committer: GitHub <noreply(a)github.com> date: 2018-11-30T11:34:47+01:00 summary: bpo-35336: Fix PYTHONCOERCECLOCALE=1 (GH-10806) Fix PYTHONCOERCECLOCALE=1 environment variable: only coerce the C locale if the LC_CTYPE locale is "C". files: A Misc/NEWS.d/next/Core and Builtins/2018-11-29-23-59-52.bpo-35336.8LOz4F.rst M Lib/test/test_c_locale_coercion.py M Python/coreconfig.c diff --git a/Lib/test/test_c_locale_coercion.py b/Lib/test/test_c_locale_coercion.py index ce8ac4eb843b..35272b5c15ac 100644 --- a/Lib/test/test_c_locale_coercion.py +++ b/Lib/test/test_c_locale_coercion.py @@ -1,11 +1,12 @@ # Tests the attempted automatic coercion of the C locale to a UTF-8 locale -import unittest import locale import os +import shutil +import subprocess import sys import sysconfig -import shutil +import unittest from collections import namedtuple from test import support @@ -25,6 +26,8 @@ # Set our expectation for the default locale used when none is specified EXPECT_COERCION_IN_DEFAULT_LOCALE = True +TARGET_LOCALES = ["C.UTF-8", "C.utf8", "UTF-8"] + # Apply some platform dependent overrides if sys.platform.startswith("linux"): if support.is_android: @@ -413,6 +416,27 @@ def test_LC_ALL_set_to_C(self): expected_warnings=[LEGACY_LOCALE_WARNING], coercion_expected=False) + def test_PYTHONCOERCECLOCALE_set_to_one(self): + # skip the test if the LC_CTYPE locale is C or coerced + old_loc = locale.setlocale(locale.LC_CTYPE, None) + self.addCleanup(locale.setlocale, locale.LC_CTYPE, old_loc) + loc = locale.setlocale(locale.LC_CTYPE, "") + if loc == "C": + self.skipTest("test requires LC_CTYPE locale different than C") + if loc in TARGET_LOCALES : + self.skipTest("coerced LC_CTYPE locale: %s" % loc) + + # bpo-35336: PYTHONCOERCECLOCALE=1 must not coerce the LC_CTYPE locale + # if it's not equal to "C" + code = 'import locale; print(locale.setlocale(locale.LC_CTYPE, None))' + env = dict(os.environ, PYTHONCOERCECLOCALE='1') + cmd = subprocess.run([sys.executable, '-c', code], + stdout=subprocess.PIPE, + env=env, + text=True) + self.assertEqual(cmd.stdout.rstrip(), loc) + + def test_main(): support.run_unittest( LocaleConfigurationTests, diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-11-29-23-59-52.bpo-35336.8LOz4F.rst b/Misc/NEWS.d/next/Core and Builtins/2018-11-29-23-59-52.bpo-35336.8LOz4F.rst new file mode 100644 index 000000000000..28f8f9bd4db7 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-11-29-23-59-52.bpo-35336.8LOz4F.rst @@ -0,0 +1,2 @@ +Fix PYTHONCOERCECLOCALE=1 environment variable: only coerce the C locale +if the LC_CTYPE locale is "C". diff --git a/Python/coreconfig.c b/Python/coreconfig.c index ad22300e56e4..2fb4e3fd196e 100644 --- a/Python/coreconfig.c +++ b/Python/coreconfig.c @@ -1061,11 +1061,17 @@ config_read_complex_options(_PyCoreConfig *config) static void config_init_locale(_PyCoreConfig *config) { - if (config->coerce_c_locale < 0) { + /* Test also if coerce_c_locale equals 1: PYTHONCOERCECLOCALE=1 doesn't + imply that the C locale is always coerced. It is only coerced if + if the LC_CTYPE locale is "C". */ + if (config->coerce_c_locale != 0) { /* The C locale enables the C locale coercion (PEP 538) */ if (_Py_LegacyLocaleDetected()) { config->coerce_c_locale = 1; } + else { + config->coerce_c_locale = 0; + } } #ifndef MS_WINDOWS @@ -1394,7 +1400,7 @@ _PyCoreConfig_Read(_PyCoreConfig *config) } } - if (config->utf8_mode < 0 || config->coerce_c_locale < 0) { + if (config->coerce_c_locale != 0 || config->utf8_mode < 0) { config_init_locale(config); }

1 0