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.

Author: Sam Martin

Lib/concurrent/futures/_base.py

@@ -404,7 +404,10 @@ def add_done_callback(self, fn):
  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.

Lib/test/test_concurrent_futures.py

@@ -1087,6 +1087,22 @@ def fn(callback_future):
  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>')

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.