Skip to content

Commit 81ed537

Browse files
authored
bpo-30320: test_eintr now uses pthread_sigmask() (#1523) (#1524)
Rewrite sigwaitinfo() and sigtimedwait() unit tests for EINTR using pthread_sigmask() to fix a race condition between the child and the parent process. Remove the pipe which was used as a weak workaround against the race condition. sigtimedwait() is now tested with a child process sending a signal instead of testing the timeout feature which is more unstable (especially regarding to clock resolution depending on the platform). (cherry picked from commit 211a392)
1 parent 418d60a commit 81ed537

File tree

1 file changed

+26
-30
lines changed

1 file changed

+26
-30
lines changed

Lib/test/eintrdata/eintr_tester.py

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -371,59 +371,55 @@ def test_sleep(self):
371371

372372

373373
@unittest.skipUnless(hasattr(signal, "setitimer"), "requires setitimer()")
374+
# bpo-30320: Need pthread_sigmask() to block the signal, otherwise the test
375+
# is vulnerable to a race condition between the child and the parent processes.
376+
@unittest.skipUnless(hasattr(signal, 'pthread_sigmask'),
377+
'need signal.pthread_sigmask()')
374378
class SignalEINTRTest(EINTRBaseTest):
375379
""" EINTR tests for the signal module. """
376380

377-
@unittest.skipUnless(hasattr(signal, 'sigtimedwait'),
378-
'need signal.sigtimedwait()')
379-
def test_sigtimedwait(self):
380-
t0 = time.monotonic()
381-
signal.sigtimedwait([signal.SIGUSR1], self.sleep_time)
382-
dt = time.monotonic() - t0
383-
self.assertGreaterEqual(dt, self.sleep_time)
384-
385-
@unittest.skipUnless(hasattr(signal, 'sigwaitinfo'),
386-
'need signal.sigwaitinfo()')
387-
def test_sigwaitinfo(self):
388-
# Issue #25277, #25868: give a few milliseconds to the parent process
389-
# between os.write() and signal.sigwaitinfo() to works around a race
390-
# condition
391-
self.sleep_time = 0.100
392-
381+
def check_sigwait(self, wait_func):
393382
signum = signal.SIGUSR1
394383
pid = os.getpid()
395384

396385
old_handler = signal.signal(signum, lambda *args: None)
397386
self.addCleanup(signal.signal, signum, old_handler)
398387

399-
rpipe, wpipe = os.pipe()
400-
401388
code = '\n'.join((
402389
'import os, time',
403390
'pid = %s' % os.getpid(),
404391
'signum = %s' % int(signum),
405392
'sleep_time = %r' % self.sleep_time,
406-
'rpipe = %r' % rpipe,
407-
'os.read(rpipe, 1)',
408-
'os.close(rpipe)',
409393
'time.sleep(sleep_time)',
410394
'os.kill(pid, signum)',
411395
))
412396

397+
old_mask = signal.pthread_sigmask(signal.SIG_BLOCK, [signum])
398+
self.addCleanup(signal.pthread_sigmask, signal.SIG_UNBLOCK, [signum])
399+
413400
t0 = time.monotonic()
414-
proc = self.subprocess(code, pass_fds=(rpipe,))
415-
os.close(rpipe)
401+
proc = self.subprocess(code)
416402
with kill_on_error(proc):
417-
# sync child-parent
418-
os.write(wpipe, b'x')
419-
os.close(wpipe)
403+
wait_func(signum)
404+
dt = time.monotonic() - t0
405+
406+
self.assertEqual(proc.wait(), 0)
420407

421-
# parent
408+
@unittest.skipUnless(hasattr(signal, 'sigwaitinfo'),
409+
'need signal.sigwaitinfo()')
410+
def test_sigwaitinfo(self):
411+
def wait_func(signum):
422412
signal.sigwaitinfo([signum])
423-
dt = time.monotonic() - t0
424-
self.assertEqual(proc.wait(), 0)
425413

426-
self.assertGreaterEqual(dt, self.sleep_time)
414+
self.check_sigwait(wait_func)
415+
416+
@unittest.skipUnless(hasattr(signal, 'sigtimedwait'),
417+
'need signal.sigwaitinfo()')
418+
def test_sigtimedwait(self):
419+
def wait_func(signum):
420+
signal.sigtimedwait([signum], 120.0)
421+
422+
self.check_sigwait(wait_func)
427423

428424

429425
@unittest.skipUnless(hasattr(signal, "setitimer"), "requires setitimer()")

0 commit comments

Comments
 (0)