Fix a regression in which a timeout could fire after being cancelled.

Closes #1148.

diff --git a/tornado/ioloop.py b/tornado/ioloop.py
index e15252d3450c779ca3b124db4da0a6840314b8ae..a8f662acb2e39aee836e88485a1fe5e5431874d2 100644 (file)
--- a/tornado/ioloop.py
+++ b/tornado/ioloop.py
@@ -754,17 +754,18 @@ class PollIOLoop(IOLoop):
                 # Do not run anything until we have determined which ones
                 # are ready, so timeouts that call add_timeout cannot
                 # schedule anything in this iteration.
+                due_timeouts = []
                 if self._timeouts:
                     now = self.time()
                     while self._timeouts:
                         if self._timeouts[0].callback is None:
-                            # the timeout was cancelled
+                            # The timeout was cancelled.  Note that the
+                            # cancellation check is repeated below for timeouts
+                            # that are cancelled by another timeout or callback.
                             heapq.heappop(self._timeouts)
                             self._cancellations -= 1
                         elif self._timeouts[0].deadline <= now:
-                            timeout = heapq.heappop(self._timeouts)
-                            callbacks.append(timeout.callback)
-                            del timeout
+                            due_timeouts.append(heapq.heappop(self._timeouts))
                         else:
                             break
                     if (self._cancellations > 512
@@ -778,9 +779,12 @@ class PollIOLoop(IOLoop):
 
                 for callback in callbacks:
                     self._run_callback(callback)
+                for timeout in due_timeouts:
+                    if timeout.callback is not None:
+                        self._run_callback(timeout.callback)
                 # Closures may be holding on to a lot of memory, so allow
                 # them to be freed before we go into our poll wait.
-                callbacks = callback = None
+                callbacks = callback = due_timeouts = timeout = None
 
                 if self._callbacks:
                     # If any callbacks or timeouts called add_callback,

diff --git a/tornado/test/ioloop_test.py b/tornado/test/ioloop_test.py
index 8bf6ee2688d487e07ac05239e1d3b8697aa133ec..110158d12443ea42bf704389d0f8ee91228aff67 100644 (file)
--- a/tornado/test/ioloop_test.py
+++ b/tornado/test/ioloop_test.py
@@ -173,6 +173,25 @@ class TestIOLoop(AsyncTestCase):
         self.io_loop.add_callback(lambda: self.io_loop.add_callback(self.stop))
         self.wait()
 
+    def test_remove_timeout_from_timeout(self):
+        calls = [False, False]
+
+        # Schedule several callbacks and wait for them all to come due at once.
+        # t2 should be cancelled by t1, even though it is already scheduled to
+        # be run before the ioloop even looks at it.
+        now = self.io_loop.time()
+        def t1():
+            calls[0] = True
+            self.io_loop.remove_timeout(t2_handle)
+        self.io_loop.add_timeout(now + 0.01, t1)
+        def t2():
+            calls[1] = True
+        t2_handle = self.io_loop.add_timeout(now + 0.02, t2)
+        self.io_loop.add_timeout(now + 0.03, self.stop)
+        time.sleep(0.03)
+        self.wait()
+        self.assertEqual(calls, [True, False])
+
     def test_timeout_with_arguments(self):
         # This tests that all the timeout methods pass through *args correctly.
         results = []