[3.7] bpo-35189: Retry fnctl calls on EINTR (GH-10413) (GH-10678)

* bpo-35189: Fix eintr_tester.py (GH-10637)

Call setitimer() before each test method, instead of once per test
case, to ensure that signals are sent in each test method.
Previously, only the first method of a testcase class got signals.

Changes:

* Replace setUpClass() with setUp() and replace tearDownClass() with
  tearDown().
* tearDown() now ensures that at least one signal has been sent.
* Replace support.run_unittest() with unittest.main() which has
  a nicer CLI and automatically discover test cases.

(cherry picked from commit aac1f81)

* bpo-35189: Retry fnctl calls on EINTR (GH-10413)

Modify the following fnctl function to retry if interrupted by a signal
(EINTR): flock, lockf, fnctl.

(cherry picked from commit b409ffa)
Co-Authored-By: nierob <[email protected]>

Author: vstinner

Lib/test/eintrdata/eintr_tester.py

@@ -10,6 +10,7 @@
 
 import contextlib
 import faulthandler
+import fcntl
 import os
 import select
 import signal
@@ -44,27 +45,32 @@ class EINTRBaseTest(unittest.TestCase):
     # 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)
+    def sighandler(self, signum, frame):
+        self.signals += 1
 
-        # Issue #25277: Use faulthandler to try to debug a hang on FreeBSD
+    def setUp(self):
+        self.signals = 0
+        self.orig_handler = signal.signal(signal.SIGALRM, self.sighandler)
+        signal.setitimer(signal.ITIMER_REAL, self.signal_delay,
+                         self.signal_period)
+
+        # Use faulthandler as watchdog to debug when a test hangs
+        # (timeout of 10 minutes)
         if hasattr(faulthandler, 'dump_traceback_later'):
             faulthandler.dump_traceback_later(10 * 60, exit=True,
                                               file=sys.__stderr__)
 
-    @classmethod
-    def stop_alarm(cls):
+    @staticmethod
+    def stop_alarm():
         signal.setitimer(signal.ITIMER_REAL, 0, 0)
 
-    @classmethod
-    def tearDownClass(cls):
-        cls.stop_alarm()
-        signal.signal(signal.SIGALRM, cls.orig_handler)
+    def tearDown(self):
+        self.stop_alarm()
+        signal.signal(signal.SIGALRM, self.orig_handler)
         if hasattr(faulthandler, 'cancel_dump_traceback_later'):
             faulthandler.cancel_dump_traceback_later()
+        # make sure that at least one signal has been received
+        self.assertGreater(self.signals, 0)
 
     def subprocess(self, *args, **kw):
         cmd_args = (sys.executable, '-c') + args
@@ -481,14 +487,43 @@ def test_devpoll(self):
         self.assertGreaterEqual(dt, self.sleep_time)
 
 
-def test_main():
-    support.run_unittest(
-        OSEINTRTest,
-        SocketEINTRTest,
-        TimeEINTRTest,
-        SignalEINTRTest,
-        SelectEINTRTest)
+class FNTLEINTRTest(EINTRBaseTest):
+    def _lock(self, lock_func, lock_name):
+        self.addCleanup(support.unlink, support.TESTFN)
+        code = '\n'.join((
+            "import fcntl, time",
+            "with open('%s', 'wb') as f:" % support.TESTFN,
+            "   fcntl.%s(f, fcntl.LOCK_EX)" % lock_name,
+            "   time.sleep(%s)" % self.sleep_time))
+        start_time = time.monotonic()
+        proc = self.subprocess(code)
+        with kill_on_error(proc):
+            with open(support.TESTFN, 'wb') as f:
+                while True:  # synchronize the subprocess
+                    dt = time.monotonic() - start_time
+                    if dt > 60.0:
+                        raise Exception("failed to sync child in %.1f sec" % dt)
+                    try:
+                        lock_func(f, fcntl.LOCK_EX | fcntl.LOCK_NB)
+                        lock_func(f, fcntl.LOCK_UN)
+                        time.sleep(0.01)
+                    except BlockingIOError:
+                        break
+                # the child locked the file just a moment ago for 'sleep_time' seconds
+                # that means that the lock below will block for 'sleep_time' minus some
+                # potential context switch delay
+                lock_func(f, fcntl.LOCK_EX)
+                dt = time.monotonic() - start_time
+                self.assertGreaterEqual(dt, self.sleep_time)
+                self.stop_alarm()
+            proc.wait()
+
+    def test_lockf(self):
+        self._lock(fcntl.lockf, "lockf")
+
+    def test_flock(self):
+        self._lock(fcntl.flock, "flock")
 
 
 if __name__ == "__main__":
-    test_main()
+    unittest.main()

Misc/NEWS.d/next/Library/2018-11-09-13-35-36.bpo-35189.gog-sl.rst

@@ -0,0 +1,2 @@
+Modify the following fnctl function to retry if interrupted by a signal
+(EINTR): flock, lockf, fnctl

Modules/fcntlmodule.c

@@ -64,6 +64,7 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg)
     char *str;
     Py_ssize_t len;
     char buf[1024];
+    int async_err = 0;
 
     if (arg != NULL) {
         int parse_result;
@@ -75,12 +76,13 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg)
                 return NULL;
             }
             memcpy(buf, str, len);
-            Py_BEGIN_ALLOW_THREADS
-            ret = fcntl(fd, code, buf);
-            Py_END_ALLOW_THREADS
+            do {
+                Py_BEGIN_ALLOW_THREADS
+                ret = fcntl(fd, code, buf);
+                Py_END_ALLOW_THREADS
+            } while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
             if (ret < 0) {
-                PyErr_SetFromErrno(PyExc_OSError);
-                return NULL;
+                return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL;
             }
             return PyBytes_FromStringAndSize(buf, len);
         }
@@ -95,12 +97,13 @@ fcntl_fcntl_impl(PyObject *module, int fd, int code, PyObject *arg)
         }
     }
 
-    Py_BEGIN_ALLOW_THREADS
-    ret = fcntl(fd, code, (int)int_arg);
-    Py_END_ALLOW_THREADS
+    do {
+        Py_BEGIN_ALLOW_THREADS
+        ret = fcntl(fd, code, (int)int_arg);
+        Py_END_ALLOW_THREADS
+    } while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
     if (ret < 0) {
-        PyErr_SetFromErrno(PyExc_OSError);
-        return NULL;
+        return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL;
     }
     return PyLong_FromLong((long)ret);
 }
@@ -283,11 +286,14 @@ fcntl_flock_impl(PyObject *module, int fd, int code)
 /*[clinic end generated code: output=84059e2b37d2fc64 input=b70a0a41ca22a8a0]*/
 {
     int ret;
+    int async_err = 0;
 
 #ifdef HAVE_FLOCK
-    Py_BEGIN_ALLOW_THREADS
-    ret = flock(fd, code);
-    Py_END_ALLOW_THREADS
+    do {
+        Py_BEGIN_ALLOW_THREADS
+        ret = flock(fd, code);
+        Py_END_ALLOW_THREADS
+    } while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
 #else
 
 #ifndef LOCK_SH
@@ -310,14 +316,15 @@ fcntl_flock_impl(PyObject *module, int fd, int code)
             return NULL;
         }
         l.l_whence = l.l_start = l.l_len = 0;
-        Py_BEGIN_ALLOW_THREADS
-        ret = fcntl(fd, (code & LOCK_NB) ? F_SETLK : F_SETLKW, &l);
-        Py_END_ALLOW_THREADS
+        do {
+            Py_BEGIN_ALLOW_THREADS
+            ret = fcntl(fd, (code & LOCK_NB) ? F_SETLK : F_SETLKW, &l);
+            Py_END_ALLOW_THREADS
+        } while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
     }
 #endif /* HAVE_FLOCK */
     if (ret < 0) {
-        PyErr_SetFromErrno(PyExc_OSError);
-        return NULL;
+        return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL;
     }
     Py_RETURN_NONE;
 }
@@ -363,6 +370,7 @@ fcntl_lockf_impl(PyObject *module, int fd, int code, PyObject *lenobj,
 /*[clinic end generated code: output=4985e7a172e7461a input=3a5dc01b04371f1a]*/
 {
     int ret;
+    int async_err = 0;
 
 #ifndef LOCK_SH
 #define LOCK_SH         1       /* shared lock */
@@ -407,13 +415,14 @@ fcntl_lockf_impl(PyObject *module, int fd, int code, PyObject *lenobj,
                 return NULL;
         }
         l.l_whence = whence;
-        Py_BEGIN_ALLOW_THREADS
-        ret = fcntl(fd, (code & LOCK_NB) ? F_SETLK : F_SETLKW, &l);
-        Py_END_ALLOW_THREADS
+        do {
+            Py_BEGIN_ALLOW_THREADS
+            ret = fcntl(fd, (code & LOCK_NB) ? F_SETLK : F_SETLKW, &l);
+            Py_END_ALLOW_THREADS
+        } while (ret == -1 && errno == EINTR && !(async_err = PyErr_CheckSignals()));
     }
     if (ret < 0) {
-        PyErr_SetFromErrno(PyExc_OSError);
-        return NULL;
+        return !async_err ? PyErr_SetFromErrno(PyExc_OSError) : NULL;
     }
     Py_RETURN_NONE;
 }