[Python-checkins] [3.7] bpo-34130: Fix 2 race conditions in test_signal (GH-8329)

[Victor Stinner]

https://github.com/python/cpython/commit/296e5720921f1837dfd09a16457ac5bba7cb1676
commit: 296e5720921f1837dfd09a16457ac5bba7cb1676
branch: 3.7
author: Victor Stinner <vstinner at redhat.com>
committer: GitHub <noreply at github.com>
date: 2018-07-18T18:49:27+02:00
summary:

[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)

files:
M Lib/test/test_signal.py

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 @@ def handler(signum, frame):
         signal.signal(signum, handler)
 
         read, write = socket.socketpair()
-        read.setblocking(False)
         write.setblocking(False)
         signal.set_wakeup_fd(write.fileno())
 
@@ -455,26 +454,51 @@ def handler(signum, frame):
         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 @@ def handler(signum, frame):
             _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 @@ def handler(signum, frame):
 
         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 @@ def handler(signum, frame):
             _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)