https://github.com/python/cpython/commit/56742f1eb05401a27499af0ccdcb4e42148... commit: 56742f1eb05401a27499af0ccdcb4e4214859fd1 branch: 3.7 author: Victor Stinner <vstinner@redhat.com> committer: GitHub <noreply@github.com> date: 2018-11-23T17:53:14+01:00 summary: [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 aac1f81eef971876ba5b1673db9ce6620311c469) * 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 b409ffa848b280c1db1b4f450bfae14f263099ac) Co-Authored-By: nierob <nierob@users.noreply.github.com> files: A Misc/NEWS.d/next/Library/2018-11-09-13-35-36.bpo-35189.gog-sl.rst M Lib/test/eintrdata/eintr_tester.py M Modules/fcntlmodule.c diff --git a/Lib/test/eintrdata/eintr_tester.py b/Lib/test/eintrdata/eintr_tester.py index 1caeafe25d9b..c2eaf0128a5f 100644 --- a/Lib/test/eintrdata/eintr_tester.py +++ b/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() diff --git a/Misc/NEWS.d/next/Library/2018-11-09-13-35-36.bpo-35189.gog-sl.rst b/Misc/NEWS.d/next/Library/2018-11-09-13-35-36.bpo-35189.gog-sl.rst new file mode 100644 index 000000000000..3408ca4eec44 --- /dev/null +++ b/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 diff --git a/Modules/fcntlmodule.c b/Modules/fcntlmodule.c index 0baaa83d2aca..163805634baf 100644 --- a/Modules/fcntlmodule.c +++ b/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; }