Skip to content

Commit

Permalink
bpo-30320: test_eintr now uses pthread_sigmask() (#1523)
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
vstinner committed May 10, 2017
1 parent 291557e commit 211a392
Showing 1 changed file with 26 additions and 35 deletions.
61 changes: 26 additions & 35 deletions Lib/test/eintrdata/eintr_tester.py
Original file line number Diff line number Diff line change
Expand Up @@ -371,64 +371,55 @@ def test_sleep(self):


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

@unittest.skipUnless(hasattr(signal, 'sigtimedwait'),
'need signal.sigtimedwait()')
def test_sigtimedwait(self):
t0 = time.monotonic()
signal.sigtimedwait([signal.SIGUSR1], self.sleep_time)
dt = time.monotonic() - t0

if sys.platform.startswith('aix'):
# On AIX, sigtimedwait(0.2) sleeps 199.8 ms
self.assertGreaterEqual(dt, self.sleep_time * 0.9)
else:
self.assertGreaterEqual(dt, self.sleep_time)

@unittest.skipUnless(hasattr(signal, 'sigwaitinfo'),
'need signal.sigwaitinfo()')
def test_sigwaitinfo(self):
# Issue #25277, #25868: give a few milliseconds to the parent process
# between os.write() and signal.sigwaitinfo() to works around a race
# condition
self.sleep_time = 0.100

def check_sigwait(self, wait_func):
signum = signal.SIGUSR1
pid = os.getpid()

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

rpipe, wpipe = os.pipe()

code = '\n'.join((
'import os, time',
'pid = %s' % os.getpid(),
'signum = %s' % int(signum),
'sleep_time = %r' % self.sleep_time,
'rpipe = %r' % rpipe,
'os.read(rpipe, 1)',
'os.close(rpipe)',
'time.sleep(sleep_time)',
'os.kill(pid, signum)',
))

old_mask = signal.pthread_sigmask(signal.SIG_BLOCK, [signum])
self.addCleanup(signal.pthread_sigmask, signal.SIG_UNBLOCK, [signum])

t0 = time.monotonic()
proc = self.subprocess(code, pass_fds=(rpipe,))
os.close(rpipe)
proc = self.subprocess(code)
with kill_on_error(proc):
# sync child-parent
os.write(wpipe, b'x')
os.close(wpipe)
wait_func(signum)
dt = time.monotonic() - t0

self.assertEqual(proc.wait(), 0)

# parent
@unittest.skipUnless(hasattr(signal, 'sigwaitinfo'),
'need signal.sigwaitinfo()')
def test_sigwaitinfo(self):
def wait_func(signum):
signal.sigwaitinfo([signum])
dt = time.monotonic() - t0
self.assertEqual(proc.wait(), 0)

self.assertGreaterEqual(dt, self.sleep_time)
self.check_sigwait(wait_func)

@unittest.skipUnless(hasattr(signal, 'sigtimedwait'),
'need signal.sigwaitinfo()')
def test_sigtimedwait(self):
def wait_func(signum):
signal.sigtimedwait([signum], 120.0)

self.check_sigwait(wait_func)


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

0 comments on commit 211a392

Please sign in to comment.