
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2018-03-20 16:12 bysamm, last changed2022-04-11 14:58 byadmin. This issue is nowclosed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 13141 | merged | samm,2019-05-06 22:25 | |
| PR 13508 | merged | miss-islington,2019-05-22 21:29 | |
| PR 13509 | closed | miss-islington,2019-05-22 21:29 | |
| Messages (4) | |||
|---|---|---|---|
| msg314151 -(view) | Author: Sam Martin (samm)* | Date: 2018-03-20 16:12 | |
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#L323add_done_callback:https://github.com/python/cpython/blob/master/Lib/concurrent/futures/_base.py#L403I 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#L1012The 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 | |||
| msg343242 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2019-05-22 21:29 | |
New changeset2a3a2ece502c05ea33c95dd0db497189e0354bfd 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 | |||
| msg343249 -(view) | Author: miss-islington (miss-islington) | Date: 2019-05-22 22:02 | |
New changesetb73c21c0be7b42de6a88d67408249c8ec46e28f7 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 | |||
| msg343279 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2019-05-23 07:40 | |
Thank you Sam for your contribution! | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:58 | admin | set | github: 77291 |
| 2019-05-23 07:40:04 | pitrou | set | status: open -> closed messages: +msg343279 stage: patch review -> resolved |
| 2019-05-22 22:02:27 | miss-islington | set | nosy: +miss-islington messages: +msg343249 |
| 2019-05-22 21:32:17 | pitrou | set | resolution: fixed versions: + Python 3.7, Python 3.8, - Python 3.6 |
| 2019-05-22 21:29:35 | miss-islington | set | pull_requests: +pull_request13425 |
| 2019-05-22 21:29:23 | miss-islington | set | pull_requests: +pull_request13424 |
| 2019-05-22 21:29:05 | pitrou | set | messages: +msg343242 |
| 2019-05-06 22:25:48 | samm | set | keywords: +patch stage: patch review pull_requests: +pull_request13054 |
| 2018-09-20 12:39:53 | xtreak | set | nosy: +xtreak |
| 2018-03-22 00:00:44 | ned.deily | set | nosy: +bquinlan,pitrou |
| 2018-03-20 16:12:43 | samm | create | |