From 18af6c798a250cb008baa2d96a8f6cfb896c2b66 Mon Sep 17 00:00:00 2001 From: Ben Darnell Date: Mon, 17 Sep 2012 00:22:39 -0700 Subject: [PATCH] Use signal.set_wakeup_fd in IOLoop to close a race with signal handlers. --- tornado/ioloop.py | 32 ++++++++++++++++++++++++++++++++ tornado/platform/common.py | 3 +++ tornado/platform/interface.py | 6 +++++- tornado/platform/posix.py | 3 +++ 4 files changed, 43 insertions(+), 1 deletion(-) diff --git a/tornado/ioloop.py b/tornado/ioloop.py index b941087c1d..6621f920e0 100644 --- a/tornado/ioloop.py +++ b/tornado/ioloop.py @@ -269,6 +269,36 @@ def start(self): return self._thread_ident = thread.get_ident() self._running = True + + # signal.set_wakeup_fd closes a race condition in event loops: + # a signal may arrive at the beginning of select/poll/etc + # before it goes into its interruptible sleep, so the signal + # will be consumed without waking the select. The solution is + # for the (C, synchronous) signal handler to write to a pipe, + # which will then be seen by select. + # + # In python's signal handling semantics, this only matters on the + # main thread (fortunately, set_wakeup_fd only works on the main + # thread and will raise a ValueError otherwise). + # + # If someone has already set a wakeup fd, we don't want to + # disturb it. This is an issue for twisted, which does its + # SIGCHILD processing in response to its own wakeup fd being + # written to. As long as the wakeup fd is registered on the IOLoop, + # the loop will still wake up and everything should work. + old_wakeup_fd = None + if hasattr(signal, 'set_wakeup_fd'): # requires python 2.6+, unix + try: + old_wakeup_fd = signal.set_wakeup_fd(self._waker.write_fileno()) + except ValueError: # non-main thread + pass + if old_wakeup_fd != -1: + # Already set, restore previous value. This is a little racy, + # but there's no clean get_wakeup_fd and in real use the + # IOLoop is just started once at the beginning. + signal.set_wakeup_fd(old_wakeup_fd) + old_wakeup_fd = None + while True: poll_timeout = 3600.0 @@ -349,6 +379,8 @@ def start(self): self._stopped = False if self._blocking_signal_threshold is not None: signal.setitimer(signal.ITIMER_REAL, 0, 0) + if old_wakeup_fd is not None: + signal.set_wakeup_fd(old_wakeup_fd) def stop(self): """Stop the loop after the current event loop iteration is complete. diff --git a/tornado/platform/common.py b/tornado/platform/common.py index 176ce2e529..39f60bd798 100644 --- a/tornado/platform/common.py +++ b/tornado/platform/common.py @@ -69,6 +69,9 @@ def __init__(self): def fileno(self): return self.reader.fileno() + def write_fileno(self): + return self.writer.fileno() + def wake(self): try: self.writer.send(b("x")) diff --git a/tornado/platform/interface.py b/tornado/platform/interface.py index 21e72cd9eb..5006f30b8f 100644 --- a/tornado/platform/interface.py +++ b/tornado/platform/interface.py @@ -39,13 +39,17 @@ class Waker(object): the ``IOLoop`` is closed, it closes its waker too. """ def fileno(self): - """Returns a file descriptor for this waker. + """Returns the read file descriptor for this waker. Must be suitable for use with ``select()`` or equivalent on the local platform. """ raise NotImplementedError() + def write_fileno(self): + """Returns the write file descriptor for this waker.""" + raise NotImplementedError() + def wake(self): """Triggers activity on the waker's file descriptor.""" raise NotImplementedError() diff --git a/tornado/platform/posix.py b/tornado/platform/posix.py index 8d674c0e23..487f97a9a7 100644 --- a/tornado/platform/posix.py +++ b/tornado/platform/posix.py @@ -48,6 +48,9 @@ def __init__(self): def fileno(self): return self.reader.fileno() + def write_fileno(self): + return self.writer.fileno() + def wake(self): try: self.writer.write(b("x"))