Improve isolation of stack contexts.

Previously, a chain of callbacks started without any contexts could
accidentally pick up other contexts based on where the callbacks
were run.  This was due to a mistaken optimization in the no-context
case; now the behavior is the same whether the initial context was empty
or not.

diff --git a/tornado/stack_context.py b/tornado/stack_context.py
index e70fe53d8b3d148649ee1453935b35bd48622158..4b490e598cf80e88bde216af8fa799d42c9e4d9f 100644 (file)
--- a/tornado/stack_context.py
+++ b/tornado/stack_context.py
@@ -198,7 +198,7 @@ def wrap(fn):
     def wrapped(*args, **kwargs):
         callback, contexts, args = args[0], args[1], args[2:]
 
-        if contexts is _state.contexts or not contexts:
+        if contexts is _state.contexts:
             callback(*args, **kwargs)
             return
         if not _state.contexts:
@@ -231,10 +231,7 @@ def wrap(fn):
                 callback(*args, **kwargs)
         else:
             callback(*args, **kwargs)
-    if _state.contexts:
-        return _StackContextWrapper(wrapped, fn, _state.contexts)
-    else:
-        return _StackContextWrapper(fn)
+    return _StackContextWrapper(wrapped, fn, _state.contexts)
 
 
 @contextlib.contextmanager

diff --git a/tornado/test/stack_context_test.py b/tornado/test/stack_context_test.py
index 1f5cb5f33a5382ed83d0d6e021f165f4fa5d9a3a..4bbba5213051560e1d483c3d9fd6b7f0b22e0a19 100644 (file)
--- a/tornado/test/stack_context_test.py
+++ b/tornado/test/stack_context_test.py
@@ -2,7 +2,7 @@
 from __future__ import absolute_import, division, with_statement
 
 from tornado.log import app_log
-from tornado.stack_context import StackContext, wrap
+from tornado.stack_context import StackContext, wrap, NullContext
 from tornado.testing import AsyncHTTPTestCase, AsyncTestCase, ExpectLog
 from tornado.test.util import unittest
 from tornado.util import b
@@ -127,5 +127,47 @@ class StackContextTest(AsyncTestCase):
         self.io_loop.add_callback(f1)
         self.wait()
 
+    def test_isolation_nonempty(self):
+        # f2 and f3 are a chain of operations started in context c1.
+        # f2 is incidentally run under context c2, but that context should
+        # not be passed along to f3.
+        def f1():
+            with StackContext(functools.partial(self.context, 'c1')):
+                wrapped = wrap(f2)
+            with StackContext(functools.partial(self.context, 'c2')):
+                wrapped()
+
+        def f2():
+            self.assertIn('c1', self.active_contexts)
+            self.io_loop.add_callback(f3)
+
+        def f3():
+            self.assertIn('c1', self.active_contexts)
+            self.assertNotIn('c2', self.active_contexts)
+            self.stop()
+
+        self.io_loop.add_callback(f1)
+        self.wait()
+
+    def test_isolation_empty(self):
+        # Similar to test_isolation_nonempty, but here the f2/f3 chain
+        # is started without any context.  Behavior should be equivalent
+        # to the nonempty case (although historically it was not)
+        def f1():
+            with NullContext():
+                wrapped = wrap(f2)
+            with StackContext(functools.partial(self.context, 'c2')):
+                wrapped()
+
+        def f2():
+            self.io_loop.add_callback(f3)
+
+        def f3():
+            self.assertNotIn('c2', self.active_contexts)
+            self.stop()
+
+        self.io_loop.add_callback(f1)
+        self.wait()
+
 if __name__ == '__main__':
     unittest.main()