From d076c53e52bc255af81a64e3476a8dd03f8f7fdf Mon Sep 17 00:00:00 2001 From: Oran Avraham Date: Sun, 25 Nov 2018 15:49:53 +0200 Subject: [PATCH 1/2] bpo-35310: Retry select() calls on EINTR even if deadline has passed select() calls are retried on EINTR (per PEP 475). However, if a timeout was provided and the deadline has passed after running signal handlers, we should still retry select() with a non-blocking call -- to make sure rlist, wlist and xlist are initialized appropriately. --- Lib/test/eintrdata/eintr_tester.py | 13 +++++++++++++ .../2018-11-25-14-53-00.bpo-35310.75QGfz.rst | 3 +++ Modules/selectmodule.c | 4 ++-- 3 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-11-25-14-53-00.bpo-35310.75QGfz.rst diff --git a/Lib/test/eintrdata/eintr_tester.py b/Lib/test/eintrdata/eintr_tester.py index c2eaf0128a5f93..e5e141123d7af2 100644 --- a/Lib/test/eintrdata/eintr_tester.py +++ b/Lib/test/eintrdata/eintr_tester.py @@ -39,6 +39,8 @@ class EINTRBaseTest(unittest.TestCase): # delay for initial signal delivery signal_delay = 0.1 + # signal handler run time duration + signal_duration = 0.05 # signal delivery periodicity signal_period = 0.1 # default sleep time for tests - should obviously have: @@ -46,6 +48,7 @@ class EINTRBaseTest(unittest.TestCase): sleep_time = 0.2 def sighandler(self, signum, frame): + time.sleep(self.signal_duration) self.signals += 1 def setUp(self): @@ -441,6 +444,16 @@ def test_select(self): self.stop_alarm() self.assertGreaterEqual(dt, self.sleep_time) + def test_select_long_signal(self): + rd, wr = os.pipe() + self.addCleanup(os.close, rd) + self.addCleanup(os.close, wr) + # timeout is enough for signal to fire, but not for it to return + timeout = self.signal_delay + self.signal_duration/2 + rlist, _, _ = select.select([rd], [], [], timeout) + self.stop_alarm() + self.assertNotIn(rd, rlist) + @unittest.skipIf(sys.platform == "darwin", "poll may fail on macOS; see issue #28087") @unittest.skipUnless(hasattr(select, 'poll'), 'need select.poll') diff --git a/Misc/NEWS.d/next/Library/2018-11-25-14-53-00.bpo-35310.75QGfz.rst b/Misc/NEWS.d/next/Library/2018-11-25-14-53-00.bpo-35310.75QGfz.rst new file mode 100644 index 00000000000000..586db9fc1f61be --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-11-25-14-53-00.bpo-35310.75QGfz.rst @@ -0,0 +1,3 @@ +Fix a bug in ``select.select()`` where a retry does not occur on EINTR if the +deadline has already passed after running the signal handlers. Patch by Oran +Avraham. diff --git a/Modules/selectmodule.c b/Modules/selectmodule.c index d86727a8978dc0..69d2a2b7eb67fa 100644 --- a/Modules/selectmodule.c +++ b/Modules/selectmodule.c @@ -335,8 +335,8 @@ select_select_impl(PyObject *module, PyObject *rlist, PyObject *wlist, if (tvp) { timeout = deadline - _PyTime_GetMonotonicClock(); if (timeout < 0) { - n = 0; - break; + timeout = 0; + /* retry select() with a non-blocking call */ } _PyTime_AsTimeval_noraise(timeout, &tv, _PyTime_ROUND_CEILING); /* retry select() with the recomputed timeout */ From 9683be9e1a8deed7921c53e7798c397164e1a778 Mon Sep 17 00:00:00 2001 From: oranav Date: Mon, 3 Dec 2018 21:37:38 +0200 Subject: [PATCH 2/2] bpo-35310: Remove EINTR test case when handler is delayed --- Lib/test/eintrdata/eintr_tester.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/Lib/test/eintrdata/eintr_tester.py b/Lib/test/eintrdata/eintr_tester.py index e5e141123d7af2..c2eaf0128a5f93 100644 --- a/Lib/test/eintrdata/eintr_tester.py +++ b/Lib/test/eintrdata/eintr_tester.py @@ -39,8 +39,6 @@ class EINTRBaseTest(unittest.TestCase): # delay for initial signal delivery signal_delay = 0.1 - # signal handler run time duration - signal_duration = 0.05 # signal delivery periodicity signal_period = 0.1 # default sleep time for tests - should obviously have: @@ -48,7 +46,6 @@ class EINTRBaseTest(unittest.TestCase): sleep_time = 0.2 def sighandler(self, signum, frame): - time.sleep(self.signal_duration) self.signals += 1 def setUp(self): @@ -444,16 +441,6 @@ def test_select(self): self.stop_alarm() self.assertGreaterEqual(dt, self.sleep_time) - def test_select_long_signal(self): - rd, wr = os.pipe() - self.addCleanup(os.close, rd) - self.addCleanup(os.close, wr) - # timeout is enough for signal to fire, but not for it to return - timeout = self.signal_delay + self.signal_duration/2 - rlist, _, _ = select.select([rd], [], [], timeout) - self.stop_alarm() - self.assertNotIn(rd, rlist) - @unittest.skipIf(sys.platform == "darwin", "poll may fail on macOS; see issue #28087") @unittest.skipUnless(hasattr(select, 'poll'), 'need select.poll')