Issue #23285: PEP 475 -- Retry system calls failing with EINTR.

5 files changed, 287 insertions, 144 deletions

diff --git a/Lib/test/eintrdata/eintr_tester.py b/Lib/test/eintrdata/eintr_tester.py
new file mode 100644
index 0000000..40dca84
--- /dev/null
+++ b/Lib/test/eintrdata/eintr_tester.py
@@ -0,0 +1,260 @@
+"""
+This test suite exercises some system calls subject to interruption with EINTR,
+to check that it is actually handled transparently.
+It is intended to be run by the main test suite within a child process, to
+ensure there is no background thread running (so that signals are delivered to
+the correct thread).
+Signals are generated in-process using setitimer(ITIMER_REAL), which allows
+sub-second periodicity (contrarily to signal()).
+"""
+
+import io
+import os
+import signal
+import socket
+import time
+import unittest
+
+from test import support
+
+
+@unittest.skipUnless(hasattr(signal, "setitimer"), "requires setitimer()")
+class EINTRBaseTest(unittest.TestCase):
+    """ Base class for EINTR tests. """
+
+    # delay for initial signal delivery
+    signal_delay = 0.1
+    # signal delivery periodicity
+    signal_period = 0.1
+    # default sleep time for tests - should obviously have:
+    # sleep_time > signal_period
+    sleep_time = 0.2
+
+    @classmethod
+    def setUpClass(cls):
+        cls.orig_handler = signal.signal(signal.SIGALRM, lambda *args: None)
+        signal.setitimer(signal.ITIMER_REAL, cls.signal_delay,
+                         cls.signal_period)
+
+    @classmethod
+    def tearDownClass(cls):
+        signal.setitimer(signal.ITIMER_REAL, 0, 0)
+        signal.signal(signal.SIGALRM, cls.orig_handler)
+
+    @classmethod
+    def _sleep(cls):
+        # default sleep time
+        time.sleep(cls.sleep_time)
+
+
+@unittest.skipUnless(hasattr(signal, "setitimer"), "requires setitimer()")
+class OSEINTRTest(EINTRBaseTest):
+    """ EINTR tests for the os module. """
+
+    def _test_wait_multiple(self, wait_func):
+        num = 3
+        for _ in range(num):
+            pid = os.fork()
+            if pid == 0:
+                self._sleep()
+                os._exit(0)
+        for _ in range(num):
+            wait_func()
+
+    def test_wait(self):
+        self._test_wait_multiple(os.wait)
+
+    @unittest.skipUnless(hasattr(os, 'wait3'), 'requires wait3()')
+    def test_wait3(self):
+        self._test_wait_multiple(lambda: os.wait3(0))
+
+    def _test_wait_single(self, wait_func):
+        pid = os.fork()
+        if pid == 0:
+            self._sleep()
+            os._exit(0)
+        else:
+            wait_func(pid)
+
+    def test_waitpid(self):
+        self._test_wait_single(lambda pid: os.waitpid(pid, 0))
+
+    @unittest.skipUnless(hasattr(os, 'wait4'), 'requires wait4()')
+    def test_wait4(self):
+        self._test_wait_single(lambda pid: os.wait4(pid, 0))
+
+    def test_read(self):
+        rd, wr = os.pipe()
+        self.addCleanup(os.close, rd)
+        # wr closed explicitly by parent
+
+        # the payload below are smaller than PIPE_BUF, hence the writes will be
+        # atomic
+        datas = [b"hello", b"world", b"spam"]
+
+        pid = os.fork()
+        if pid == 0:
+            os.close(rd)
+            for data in datas:
+                # let the parent block on read()
+                self._sleep()
+                os.write(wr, data)
+            os._exit(0)
+        else:
+            self.addCleanup(os.waitpid, pid, 0)
+            os.close(wr)
+            for data in datas:
+                self.assertEqual(data, os.read(rd, len(data)))
+
+    def test_write(self):
+        rd, wr = os.pipe()
+        self.addCleanup(os.close, wr)
+        # rd closed explicitly by parent
+
+        # we must write enough data for the write() to block
+        data = b"xyz" * support.PIPE_MAX_SIZE
+
+        pid = os.fork()
+        if pid == 0:
+            os.close(wr)
+            read_data = io.BytesIO()
+            # let the parent block on write()
+            self._sleep()
+            while len(read_data.getvalue()) < len(data):
+                chunk = os.read(rd, 2 * len(data))
+                read_data.write(chunk)
+            self.assertEqual(read_data.getvalue(), data)
+            os._exit(0)
+        else:
+            os.close(rd)
+            written = 0
+            while written < len(data):
+                written += os.write(wr, memoryview(data)[written:])
+            self.assertEqual(0, os.waitpid(pid, 0)[1])
+
+
+@unittest.skipUnless(hasattr(signal, "setitimer"), "requires setitimer()")
+class SocketEINTRTest(EINTRBaseTest):
+    """ EINTR tests for the socket module. """
+
+    @unittest.skipUnless(hasattr(socket, 'socketpair'), 'needs socketpair()')
+    def _test_recv(self, recv_func):
+        rd, wr = socket.socketpair()
+        self.addCleanup(rd.close)
+        # wr closed explicitly by parent
+
+        # single-byte payload guard us against partial recv
+        datas = [b"x", b"y", b"z"]
+
+        pid = os.fork()
+        if pid == 0:
+            rd.close()
+            for data in datas:
+                # let the parent block on recv()
+                self._sleep()
+                wr.sendall(data)
+            os._exit(0)
+        else:
+            self.addCleanup(os.waitpid, pid, 0)
+            wr.close()
+            for data in datas:
+                self.assertEqual(data, recv_func(rd, len(data)))
+
+    def test_recv(self):
+        self._test_recv(socket.socket.recv)
+
+    @unittest.skipUnless(hasattr(socket.socket, 'recvmsg'), 'needs recvmsg()')
+    def test_recvmsg(self):
+        self._test_recv(lambda sock, data: sock.recvmsg(data)[0])
+
+    def _test_send(self, send_func):
+        rd, wr = socket.socketpair()
+        self.addCleanup(wr.close)
+        # rd closed explicitly by parent
+
+        # we must send enough data for the send() to block
+        data = b"xyz" * (support.SOCK_MAX_SIZE // 3)
+
+        pid = os.fork()
+        if pid == 0:
+            wr.close()
+            # let the parent block on send()
+            self._sleep()
+            received_data = bytearray(len(data))
+            n = 0
+            while n < len(data):
+                n += rd.recv_into(memoryview(received_data)[n:])
+            self.assertEqual(received_data, data)
+            os._exit(0)
+        else:
+            rd.close()
+            written = 0
+            while written < len(data):
+                sent = send_func(wr, memoryview(data)[written:])
+                # sendall() returns None
+                written += len(data) if sent is None else sent
+            self.assertEqual(0, os.waitpid(pid, 0)[1])
+
+    def test_send(self):
+        self._test_send(socket.socket.send)
+
+    def test_sendall(self):
+        self._test_send(socket.socket.sendall)
+
+    @unittest.skipUnless(hasattr(socket.socket, 'sendmsg'), 'needs sendmsg()')
+    def test_sendmsg(self):
+        self._test_send(lambda sock, data: sock.sendmsg([data]))
+
+    def test_accept(self):
+        sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+        self.addCleanup(sock.close)
+
+        sock.bind((support.HOST, 0))
+        _, port = sock.getsockname()
+        sock.listen()
+
+        pid = os.fork()
+        if pid == 0:
+            # let parent block on accept()
+            self._sleep()
+            with socket.create_connection((support.HOST, port)):
+                self._sleep()
+            os._exit(0)
+        else:
+            self.addCleanup(os.waitpid, pid, 0)
+            client_sock, _ = sock.accept()
+            client_sock.close()
+
+    @unittest.skipUnless(hasattr(os, 'mkfifo'), 'needs mkfifo()')
+    def _test_open(self, do_open_close_reader, do_open_close_writer):
+        # Use a fifo: until the child opens it for reading, the parent will
+        # block when trying to open it for writing.
+        support.unlink(support.TESTFN)
+        os.mkfifo(support.TESTFN)
+        self.addCleanup(support.unlink, support.TESTFN)
+
+        pid = os.fork()
+        if pid == 0:
+            # let the parent block
+            self._sleep()
+            do_open_close_reader(support.TESTFN)
+            os._exit(0)
+        else:
+            self.addCleanup(os.waitpid, pid, 0)
+            do_open_close_writer(support.TESTFN)
+
+    def test_open(self):
+        self._test_open(lambda path: open(path, 'r').close(),
+                        lambda path: open(path, 'w').close())
+
+    def test_os_open(self):
+        self._test_open(lambda path: os.close(os.open(path, os.O_RDONLY)),
+                        lambda path: os.close(os.open(path, os.O_WRONLY)))
+
+
+def test_main():
+    support.run_unittest(OSEINTRTest, SocketEINTRTest)
+
+
+if __name__ == "__main__":
+    test_main()
diff --git a/Lib/test/test_eintr.py b/Lib/test/test_eintr.py
new file mode 100644
index 0000000..8b5f507
--- /dev/null
+++ b/Lib/test/test_eintr.py
@@ -0,0 +1,20 @@
+import os
+import signal
+import unittest
+
+from test import script_helper, support
+
+
+@unittest.skipUnless(os.name == "posix", "only supported on Unix")
+class EINTRTests(unittest.TestCase):
+
+    @unittest.skipUnless(hasattr(signal, "setitimer"), "requires setitimer()")
+    def test_all(self):
+        # Run the tester in a sub-process, to make sure there is only one
+        # thread (for reliable signal delivery).
+        tester = support.findfile("eintr_tester.py", subdir="eintrdata")
+        script_helper.assert_python_ok(tester)
+
+
+if __name__ == "__main__":
+    unittest.main()
diff --git a/Lib/test/test_signal.py b/Lib/test/test_signal.py
index 8f3dde4..774fc80 100644
--- a/Lib/test/test_signal.py
+++ b/Lib/test/test_signal.py
@@ -587,7 +587,7 @@ class SiginterruptTest(unittest.TestCase):
             r, w = os.pipe()
 
             def handler(signum, frame):
-                pass
+                1 / 0
 
             signal.signal(signal.SIGALRM, handler)
             if interrupt is not None:
@@ -604,9 +604,8 @@ class SiginterruptTest(unittest.TestCase):
                     try:
                         # blocking call: read from a pipe without data
                         os.read(r, 1)
-                    except OSError as err:
-                        if err.errno != errno.EINTR:
-                            raise
+                    except ZeroDivisionError:
+                        pass
                     else:
                         sys.exit(2)
                 sys.exit(3)
diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py
index 9cd9dad..f7cacf0 100644
--- a/Lib/test/test_socket.py
+++ b/Lib/test/test_socket.py
@@ -3590,7 +3590,7 @@ class InterruptedTimeoutBase(unittest.TestCase):
     def setUp(self):
         super().setUp()
         orig_alrm_handler = signal.signal(signal.SIGALRM,
-                                          lambda signum, frame: None)
+                                          lambda signum, frame: 1 / 0)
         self.addCleanup(signal.signal, signal.SIGALRM, orig_alrm_handler)
         self.addCleanup(self.setAlarm, 0)
 
@@ -3627,13 +3627,11 @@ class InterruptedRecvTimeoutTest(InterruptedTimeoutBase, UDPTestBase):
         self.serv.settimeout(self.timeout)
 
     def checkInterruptedRecv(self, func, *args, **kwargs):
-        # Check that func(*args, **kwargs) raises OSError with an
+        # Check that func(*args, **kwargs) raises 
         # errno of EINTR when interrupted by a signal.
         self.setAlarm(self.alarm_time)
-        with self.assertRaises(OSError) as cm:
+        with self.assertRaises(ZeroDivisionError) as cm:
             func(*args, **kwargs)
-        self.assertNotIsInstance(cm.exception, socket.timeout)
-        self.assertEqual(cm.exception.errno, errno.EINTR)
 
     def testInterruptedRecvTimeout(self):
         self.checkInterruptedRecv(self.serv.recv, 1024)
@@ -3689,12 +3687,10 @@ class InterruptedSendTimeoutTest(InterruptedTimeoutBase,
         # Check that func(*args, **kwargs), run in a loop, raises
         # OSError with an errno of EINTR when interrupted by a
         # signal.
-        with self.assertRaises(OSError) as cm:
+        with self.assertRaises(ZeroDivisionError) as cm:
             while True:
                 self.setAlarm(self.alarm_time)
                 func(*args, **kwargs)
-        self.assertNotIsInstance(cm.exception, socket.timeout)
-        self.assertEqual(cm.exception.errno, errno.EINTR)
 
     # Issue #12958: The following tests have problems on OS X prior to 10.7
     @support.requires_mac_ver(10, 7)
@@ -4062,117 +4058,6 @@ class FileObjectClassTestCase(SocketConnectedTest):
         pass
 
 
-class FileObjectInterruptedTestCase(unittest.TestCase):
-    """Test that the file object correctly handles EINTR internally."""
-
-    class MockSocket(object):
-        def __init__(self, recv_funcs=()):
-            # A generator that returns callables that we'll call for each
-            # call to recv().
-            self._recv_step = iter(recv_funcs)
-
-        def recv_into(self, buffer):
-            data = next(self._recv_step)()
-            assert len(buffer) >= len(data)
-            buffer[:len(data)] = data
-            return len(data)
-
-        def _decref_socketios(self):
-            pass
-
-        def _textiowrap_for_test(self, buffering=-1):
-            raw = socket.SocketIO(self, "r")
-            if buffering < 0:
-                buffering = io.DEFAULT_BUFFER_SIZE
-            if buffering == 0:
-                return raw
-            buffer = io.BufferedReader(raw, buffering)
-            text = io.TextIOWrapper(buffer, None, None)
-            text.mode = "rb"
-            return text
-
-    @staticmethod
-    def _raise_eintr():
-        raise OSError(errno.EINTR, "interrupted")
-
-    def _textiowrap_mock_socket(self, mock, buffering=-1):
-        raw = socket.SocketIO(mock, "r")
-        if buffering < 0:
-            buffering = io.DEFAULT_BUFFER_SIZE
-        if buffering == 0:
-            return raw
-        buffer = io.BufferedReader(raw, buffering)
-        text = io.TextIOWrapper(buffer, None, None)
-        text.mode = "rb"
-        return text
-
-    def _test_readline(self, size=-1, buffering=-1):
-        mock_sock = self.MockSocket(recv_funcs=[
-                lambda : b"This is the first line\nAnd the sec",
-                self._raise_eintr,
-                lambda : b"ond line is here\n",
-                lambda : b"",
-                lambda : b"",  # XXX(gps): io library does an extra EOF read
-            ])
-        fo = mock_sock._textiowrap_for_test(buffering=buffering)
-        self.assertEqual(fo.readline(size), "This is the first line\n")
-        self.assertEqual(fo.readline(size), "And the second line is here\n")
-
-    def _test_read(self, size=-1, buffering=-1):
-        mock_sock = self.MockSocket(recv_funcs=[
-                lambda : b"This is the first line\nAnd the sec",
-                self._raise_eintr,
-                lambda : b"ond line is here\n",
-                lambda : b"",
-                lambda : b"",  # XXX(gps): io library does an extra EOF read
-            ])
-        expecting = (b"This is the first line\n"
-                     b"And the second line is here\n")
-        fo = mock_sock._textiowrap_for_test(buffering=buffering)
-        if buffering == 0:
-            data = b''
-        else:
-            data = ''
-            expecting = expecting.decode('utf-8')
-        while len(data) != len(expecting):
-            part = fo.read(size)
-            if not part:
-                break
-            data += part
-        self.assertEqual(data, expecting)
-
-    def test_default(self):
-        self._test_readline()
-        self._test_readline(size=100)
-        self._test_read()
-        self._test_read(size=100)
-
-    def test_with_1k_buffer(self):
-        self._test_readline(buffering=1024)
-        self._test_readline(size=100, buffering=1024)
-        self._test_read(buffering=1024)
-        self._test_read(size=100, buffering=1024)
-
-    def _test_readline_no_buffer(self, size=-1):
-        mock_sock = self.MockSocket(recv_funcs=[
-                lambda : b"a",
-                lambda : b"\n",
-                lambda : b"B",
-                self._raise_eintr,
-                lambda : b"b",
-                lambda : b"",
-            ])
-        fo = mock_sock._textiowrap_for_test(buffering=0)
-        self.assertEqual(fo.readline(size), b"a\n")
-        self.assertEqual(fo.readline(size), b"Bb")
-
-    def test_no_buffer(self):
-        self._test_readline_no_buffer()
-        self._test_readline_no_buffer(size=4)
-        self._test_read(buffering=0)
-        self._test_read(size=100, buffering=0)
-
-
 class UnbufferedFileObjectClassTestCase(FileObjectClassTestCase):
 
     """Repeat the tests from FileObjectClassTestCase with bufsize==0.
@@ -5388,7 +5273,6 @@ def test_main():
     tests.extend([
         NonBlockingTCPTests,
         FileObjectClassTestCase,
-        FileObjectInterruptedTestCase,
         UnbufferedFileObjectClassTestCase,
         LineBufferedFileObjectClassTestCase,
         SmallBufferedFileObjectClassTestCase,
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index 5381115..3d34859 100644
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -2421,25 +2421,6 @@ class ProcessTestCaseNoPoll(ProcessTestCase):
         ProcessTestCase.tearDown(self)
 
 
-class HelperFunctionTests(unittest.TestCase):
-    @unittest.skipIf(mswindows, "errno and EINTR make no sense on windows")
-    def test_eintr_retry_call(self):
-        record_calls = []
-        def fake_os_func(*args):
-            record_calls.append(args)
-            if len(record_calls) == 2:
-                raise OSError(errno.EINTR, "fake interrupted system call")
-            return tuple(reversed(args))
-
-        self.assertEqual((999, 256),
-                         subprocess._eintr_retry_call(fake_os_func, 256, 999))
-        self.assertEqual([(256, 999)], record_calls)
-        # This time there will be an EINTR so it will loop once.
-        self.assertEqual((666,),
-                         subprocess._eintr_retry_call(fake_os_func, 666))
-        self.assertEqual([(256, 999), (666,), (666,)], record_calls)
-
-
 @unittest.skipUnless(mswindows, "Windows-specific tests")
 class CommandsWithSpaces (BaseTestCase):
 
@@ -2528,7 +2509,6 @@ def test_main():
                   Win32ProcessTestCase,
                   CommandTests,
                   ProcessTestCaseNoPoll,
-                  HelperFunctionTests,
                   CommandsWithSpaces,
                   ContextManagerTests,
                   )