From: Ben Darnell <ben@bendarnell.com>
Date: Sat, 23 Feb 2013 17:53:04 +0000 (-0500)
Subject: If a @return_future function raises immediately, don't run the callback.
X-Git-Tag: v3.0.0~96
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5d230aa01b0cfa9185ee75c408dd5b1b81b34f20;p=thirdparty%2Ftornado.git

If a @return_future function raises immediately, don't run the callback.
---

diff --git a/tornado/concurrent.py b/tornado/concurrent.py
index edef8c8c0..19f1c87be 100644
--- a/tornado/concurrent.py
+++ b/tornado/concurrent.py
@@ -150,8 +150,6 @@ def return_future(f):
         future = Future()
         callback, args, kwargs = replacer.replace(future.set_result,
                                                   args, kwargs)
-        if callback is not None:
-            future.add_done_callback(wrap(callback))
 
         def handle_error(typ, value, tb):
             future.set_exception(value)
@@ -172,6 +170,15 @@ def return_future(f):
             # go ahead and raise it to the caller directly without waiting
             # for them to inspect the Future.
             raise_exc_info(exc_info)
+
+        # If the caller passed in a callback, schedule it to be called
+        # when the future resolves.  It is important that this happens
+        # just before we return the future, or else we risk confusing
+        # stack contexts with multiple exceptions (one here with the
+        # immediate exception, and again when the future resolves and
+        # the callback triggers its exception by calling future.result()).
+        if callback is not None:
+            future.add_done_callback(wrap(callback))
         return future
     return wrapper
 
diff --git a/tornado/test/concurrent_test.py b/tornado/test/concurrent_test.py
index e6fd1d7ce..f58e5c062 100644
--- a/tornado/test/concurrent_test.py
+++ b/tornado/test/concurrent_test.py
@@ -55,17 +55,14 @@ class ReturnFutureTest(AsyncTestCase):
         with self.assertRaises(ZeroDivisionError):
             # The caller sees the error just like a normal function.
             self.immediate_failure(callback=self.stop)
-        # The callback is also run, with a future that contains the error.
-        future = self.wait()
-        with self.assertRaises(ZeroDivisionError):
-            future.result()
+        # The callback is not run because the function failed synchronously.
+        self.io_loop.add_timeout(self.io_loop.time() + 0.05, self.stop)
+        result = self.wait()
+        self.assertIs(result, None)
 
     def test_return_value(self):
         with self.assertRaises(ReturnValueIgnoredError):
             self.return_value(callback=self.stop)
-        future = self.wait()
-        with self.assertRaises(ReturnValueIgnoredError):
-            future.result()
 
     def test_callback_kw(self):
         future = self.sync_future(callback=self.stop)