From: Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com>
Date: Wed, 22 May 2019 22:02:24 +0000 (-0700)
Subject: bpo-33110: Catch errors raised when running add_done_callback on already completed... 
X-Git-Tag: v3.7.4rc1~113
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b73c21c0be7b42de6a88d67408249c8ec46e28f7;p=thirdparty%2FPython%2Fcpython.git

bpo-33110: Catch errors raised when running add_done_callback on already completed futures (GH-13141)


Wrap the callback call within the `add_done_callback` function within concurrent.futures, in order to behave in an identical manner to callbacks added to a running future are triggered once it has completed.
(cherry picked from commit 2a3a2ece502c05ea33c95dd0db497189e0354bfd)

Co-authored-by: Sam Martin <ABitMoreDepth@users.noreply.github.com>
---

diff --git a/Lib/concurrent/futures/_base.py b/Lib/concurrent/futures/_base.py
index 0b847307a328..46d30a50065e 100644
--- a/Lib/concurrent/futures/_base.py
+++ b/Lib/concurrent/futures/_base.py
@@ -400,7 +400,10 @@ class Future(object):
             if self._state not in [CANCELLED, CANCELLED_AND_NOTIFIED, FINISHED]:
                 self._done_callbacks.append(fn)
                 return
-        fn(self)
+        try:
+            fn(self)
+        except Exception:
+            LOGGER.exception('exception calling callback for %r', self)
 
     def result(self, timeout=None):
         """Return the result of the call that the future represents.
diff --git a/Lib/test/test_concurrent_futures.py b/Lib/test/test_concurrent_futures.py
index 59aa0f46f59f..add2bfdd5abe 100644
--- a/Lib/test/test_concurrent_futures.py
+++ b/Lib/test/test_concurrent_futures.py
@@ -1079,6 +1079,22 @@ class FutureTests(BaseTestCase):
         f.add_done_callback(fn)
         self.assertTrue(was_cancelled)
 
+    def test_done_callback_raises_already_succeeded(self):
+        with test.support.captured_stderr() as stderr:
+            def raising_fn(callback_future):
+                raise Exception('doh!')
+
+            f = Future()
+
+            # Set the result first to simulate a future that runs instantly,
+            # effectively allowing the callback to be run immediately.
+            f.set_result(5)
+            f.add_done_callback(raising_fn)
+
+            self.assertIn('exception calling callback for', stderr.getvalue())
+            self.assertIn('doh!', stderr.getvalue())
+
+
     def test_repr(self):
         self.assertRegex(repr(PENDING_FUTURE),
                          '<Future at 0x[0-9a-f]+ state=pending>')
diff --git a/Misc/NEWS.d/next/Library/2019-05-06-22-34-47.bpo-33110.rSJSCh.rst b/Misc/NEWS.d/next/Library/2019-05-06-22-34-47.bpo-33110.rSJSCh.rst
new file mode 100644
index 000000000000..f1e2460c4927
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2019-05-06-22-34-47.bpo-33110.rSJSCh.rst
@@ -0,0 +1 @@
+Handle exceptions raised by functions added by concurrent.futures add_done_callback correctly when the Future has already completed.