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

Fri Nov 23 13:00:19 EST 2018

https://github.com/python/cpython/commit/eef813b1091f4baaaa7411aa58d2746a9761ee99
commit: eef813b1091f4baaaa7411aa58d2746a9761ee99
branch: 3.6
author: Victor Stinner <vstinner at redhat.com>
committer: GitHub <noreply at github.com>
date: 2018年11月23日T19:00:16+01:00
summary:
[3.7] bpo-35189: Retry fnctl calls on EINTR (GH-10413) (GH-10678) (GH-10685)
* 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 at users.noreply.github.com>
(cherry picked from commit 56742f1eb05401a27499af0ccdcb4e4214859fd1)
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 8875b264b507..b5eea4986e0a 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_IOError);
- return NULL;
+ return !async_err ? PyErr_SetFromErrno(PyExc_IOError) : 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_IOError);
- return NULL;
+ return !async_err ? PyErr_SetFromErrno(PyExc_IOError) : 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_IOError);
- return NULL;
+ return !async_err ? PyErr_SetFromErrno(PyExc_IOError) : 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_IOError);
- return NULL;
+ return !async_err ? PyErr_SetFromErrno(PyExc_IOError) : NULL;
 }
 Py_RETURN_NONE;
 }
