[issue33110] Adding a done callback to a concurrent.futures Future once it has already completed, may raise an exception, contrary to docs

[Antoine Pitrou]

Antoine Pitrou  added the comment:

Thank you Sam for your contribution!

--
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33110] Adding a done callback to a concurrent.futures Future once it has already completed, may raise an exception, contrary to docs

[miss-islington]

miss-islington  added the comment:


New changeset b73c21c0be7b42de6a88d67408249c8ec46e28f7 by Miss Islington (bot) 
in branch '3.7':
bpo-33110: Catch errors raised when running add_done_callback on already 
completed futures (GH-13141)
https://github.com/python/cpython/commit/b73c21c0be7b42de6a88d67408249c8ec46e28f7


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33110] Adding a done callback to a concurrent.futures Future once it has already completed, may raise an exception, contrary to docs

[Antoine Pitrou]

Change by Antoine Pitrou :


--
resolution:  -> fixed
versions: +Python 3.7, Python 3.8 -Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33110] Adding a done callback to a concurrent.futures Future once it has already completed, may raise an exception, contrary to docs

[miss-islington]

Change by miss-islington :


--
pull_requests: +13425

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33110] Adding a done callback to a concurrent.futures Future once it has already completed, may raise an exception, contrary to docs

[Antoine Pitrou]

Antoine Pitrou  added the comment:


New changeset 2a3a2ece502c05ea33c95dd0db497189e0354bfd by Antoine Pitrou (Sam 
Martin) in branch 'master':
bpo-33110: Catch errors raised when running add_done_callback on already 
completed futures (GH-13141)
https://github.com/python/cpython/commit/2a3a2ece502c05ea33c95dd0db497189e0354bfd


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33110] Adding a done callback to a concurrent.futures Future once it has already completed, may raise an exception, contrary to docs

[miss-islington]

Change by miss-islington :


--
pull_requests: +13424

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33110] Adding a done callback to a concurrent.futures Future once it has already completed, may raise an exception, contrary to docs

[Sam Martin]

Change by Sam Martin :


--
keywords: +patch
pull_requests: +13054
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33110] Adding a done callback to a concurrent.futures Future once it has already completed, may raise an exception, contrary to docs

[Karthikeyan Singaravelan]

Change by Karthikeyan Singaravelan :


--
nosy: +xtreak

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33110] Adding a done callback to a concurrent.futures Future once it has already completed, may raise an exception, contrary to docs

[Ned Deily]

Change by Ned Deily :


--
nosy: +bquinlan, pitrou

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33110] Adding a done callback to a concurrent.futures Future once it has already completed, may raise an exception, contrary to docs

[Sam Martin]

New submission from Sam Martin :

Whilst working with concurrent.futures and ThreadPoolExecutors, my colleague 
and I have noted some undocumented behaviour.

When adding a done_callback to a future that has already completed, we note 
that that callback is executed directly, outside of any try...except statement. 
 However, both the docs and the _invoke_callbacks methods will wrap any 
done_callbacks in a try...except statement which logs the result and returns.

Could we either update the documentation to mention that callback behaviour may 
raise if added to an already completed future, or simply add the same 
try...except wrapping that the _invoke_callback method already uses please? 
(Preferably the latter)

The two pieces of the futures library I am referring to can be viewed here:
_invoke_callbacks: 
https://github.com/python/cpython/blob/master/Lib/concurrent/futures/_base.py#L323

add_done_callback: 
https://github.com/python/cpython/blob/master/Lib/concurrent/futures/_base.py#L403

I would note, that the test code which covers this area of the code, doesn't 
currently exercise this particular condition.  The closest test I could find is 
test_done_callback_already_failed, which checks that a callback can retrieve an 
exception from a future, but it does not validate what happens when a callback 
raises when the future it is attached to is already complete.
Source: 
https://github.com/python/cpython/blob/c3d9508ff22ece9a96892b628dd5813e2fb0cd80/Lib/test/test_concurrent_futures.py#L1012

The other test closely related is test_done_callback_raises, however this 
doesn't check the behaviour of a callback when added to an already completed 
future.  We should be able to simulate this by moving the f.set_result line to 
above the f.add_done_callback lines?

Source: 
https://github.com/python/cpython/blob/c3d9508ff22ece9a96892b628dd5813e2fb0cd80/Lib/test/test_concurrent_futures.py#L990

--
components: Library (Lib)
messages: 314151
nosy: samm
priority: normal
severity: normal
status: open
title: Adding a done callback to a concurrent.futures Future once it has 
already completed, may raise an exception, contrary to docs
type: behavior
versions: Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com