From 296e5720921f1837dfd09a16457ac5bba7cb1676 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 18 Jul 2018 18:49:27 +0200 Subject: [PATCH] [3.7] bpo-34130: Fix 2 race conditions in test_signal (GH-8329) * bpo-34130: Fix test_signal.test_socket() (GH-8326) test_signal.test_socket(): On Windows, sometimes even if the C signal handler succeed to write the signal number into the write end of the socketpair, the test fails with a BlockingIOError on the non-blocking read.recv(1) because the read end of the socketpair didn't receive the byte yet. Fix the race condition on Windows by setting the read end as blocking. (cherry picked from commit 99bb6df66a42625367c4f38e6802c8bb527baf4a) * bpo-34130: Fix test_signal.test_warn_on_full_buffer() (GH-8327) On Windows, sometimes test_signal.test_warn_on_full_buffer() fails to fill the socketpair buffer. In that case, the C signal handler succeed to write into the socket, it doesn't log the expected send error, and so the test fail. On Windows, the test now uses a timeout of 50 ms to fill the socketpair buffer to fix this race condition. Other changes: * Begin with large chunk size to fill the buffer to speed up the test. * Add error messages to assertion errors to more easily identify which assertion failed. * Don't set the read end of the socketpair as non-blocking. (cherry picked from commit 686b4b5ff219ed66714f3b811815776dafadc23b) --- Lib/test/test_signal.py | 57 +++++++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 16 deletions(-) diff --git a/Lib/test/test_signal.py b/Lib/test/test_signal.py index 48b7a392e50b..24cab0f89ee7 100644 --- a/Lib/test/test_signal.py +++ b/Lib/test/test_signal.py @@ -367,7 +367,6 @@ class WakeupSocketSignalTests(unittest.TestCase): signal.signal(signum, handler) read, write = socket.socketpair() - read.setblocking(False) write.setblocking(False) signal.set_wakeup_fd(write.fileno()) @@ -455,26 +454,51 @@ class WakeupSocketSignalTests(unittest.TestCase): signal.signal(signum, handler) read, write = socket.socketpair() - read.setblocking(False) - write.setblocking(False) - # Fill the send buffer + # Fill the socketpair buffer + if sys.platform == 'win32': + # bpo-34130: On Windows, sometimes non-blocking send fails to fill + # the full socketpair buffer, so use a timeout of 50 ms instead. + write.settimeout(0.050) + else: + write.setblocking(False) + + # Start with large chunk size to reduce the + # number of send needed to fill the buffer. + written = 0 + for chunk_size in (2 ** 16, 2 ** 8, 1): + chunk = b"x" * chunk_size + try: + while True: + write.send(chunk) + written += chunk_size + except (BlockingIOError, socket.timeout): + pass + + print(f"%s bytes written into the socketpair" % written, flush=True) + + write.setblocking(False) try: - while True: - write.send(b"x") + write.send(b"x") except BlockingIOError: + # The socketpair buffer seems full pass + else: + raise AssertionError("%s bytes failed to fill the socketpair " + "buffer" % written) # By default, we get a warning when a signal arrives + msg = ('Exception ignored when trying to {action} ' + 'to the signal wakeup fd') signal.set_wakeup_fd(write.fileno()) with captured_stderr() as err: _testcapi.raise_signal(signum) err = err.getvalue() - if ('Exception ignored when trying to {action} to the signal wakeup fd' - not in err): - raise AssertionError(err) + if msg not in err: + raise AssertionError("first set_wakeup_fd() test failed, " + "stderr: %r" % err) # And also if warn_on_full_buffer=True signal.set_wakeup_fd(write.fileno(), warn_on_full_buffer=True) @@ -483,9 +507,9 @@ class WakeupSocketSignalTests(unittest.TestCase): _testcapi.raise_signal(signum) err = err.getvalue() - if ('Exception ignored when trying to {action} to the signal wakeup fd' - not in err): - raise AssertionError(err) + if msg not in err: + raise AssertionError("set_wakeup_fd(warn_on_full_buffer=True) " + "test failed, stderr: %r" % err) # But not if warn_on_full_buffer=False signal.set_wakeup_fd(write.fileno(), warn_on_full_buffer=False) @@ -495,7 +519,8 @@ class WakeupSocketSignalTests(unittest.TestCase): err = err.getvalue() if err != "": - raise AssertionError("got unexpected output %r" % (err,)) + raise AssertionError("set_wakeup_fd(warn_on_full_buffer=False) " + "test failed, stderr: %r" % err) # And then check the default again, to make sure warn_on_full_buffer # settings don't leak across calls. @@ -505,9 +530,9 @@ class WakeupSocketSignalTests(unittest.TestCase): _testcapi.raise_signal(signum) err = err.getvalue() - if ('Exception ignored when trying to {action} to the signal wakeup fd' - not in err): - raise AssertionError(err) + if msg not in err: + raise AssertionError("second set_wakeup_fd() test failed, " + "stderr: %r" % err) """.format(action=action) assert_python_ok('-c', code) -- 2.47.3