classification
Title: `concurrent.futures.Future.set_running_or_notify_cancel` does not notify cancel
Type: Stage:
Components: Documentation, Library (Lib) Versions: Python 3.5, Python 3.2, Python 3.3, Python 3.4
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: docs@python Nosy List: bquinlan, bwhmather, docs@python, pitrou
Priority: normal Keywords: patch

Created on 2014-10-14 09:05 by bwhmather, last changed 2017-10-03 09:40 by pitrou.

Files
File name Uploaded Description Edit
change-callbacks.patch bwhmather, 2014-10-14 09:05 Patch to change callbacks to match docs review
change-docs.patch bwhmather, 2014-10-14 09:06 Fixes the documentation to match the current behaviour. review
change-docs-and-waiters.patch bwhmather, 2014-10-14 09:07 in addition to fixing the documentation, this also fixes the inconsistency between waiters and callbacks by invoking waiters' `add_cancelled` method in `cancel`. review
non-magic-waiters.patch bwhmather, 2014-10-25 20:44 review
Messages (3)
msg229282 - (view) Author: Ben Mather (bwhmather) * Date: 2014-10-14 09:05
The documentation for the `set_running_or_notify_cancel` method on `concurrent.futures.Future` states that it will notify waiting threads and trigger callbacks after the `Future` has been cancelled, however currently this is done immediately by the call to `cancel`.

Oddly waiters (private interface used to implement `wait` and `as_completed`) do follow the behaviour documented for callbacks (they are called in `set_running_or_notify_cancel`) which means that they are not equivalent to setting a callback on each future.


I have attached three possible patches:
 1) "change-callbacks.patch" - this changes the behaviour to match the documentation.
 2) "change-docs.patch" - this fixes the documentation to match the current behaviour.
 3) "change-docs-and-waiters.patch" - in addition to fixing the documentation, this also fixes the inconsistency between waiters and callbacks by invoking waiters' `add_cancelled` method in `cancel`.

I believe moving to the documented behaviour (1) would be a mistake as currently `set_running_or_notify_cancel` and the `RUNNING` state can be skipped entirely allowing Futures to be used just as a way to send results.

Should mention that I have a separate patch (which I will submit separately (or here?)) that re-implements `wait` and `as_completed` using only publicly exposed methods.  This makes it possible to extend or replace `Future` without having to preserve its private interface.  Unfortunately this slightly breaks compatibility due to the difference between waiters and callbacks.  I thought it would be best to discuss this issue first as I don't believe the current behaviour is as intended.
msg230017 - (view) Author: Ben Mather (bwhmather) * Date: 2014-10-25 20:44
Have uploaded patch to re-implement `wait` and `as_completed` using callbacks.  See issue 22729.  Sorry for sitting on it for so long.
msg303588 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-10-03 09:40
"change-docs-and-waiters.patch" looks reasonable on the principle, but I doubt it still applies.

Also, we use Github nowadays.  You may still upload patches if you prefer, but pull requests have become the recommended way to submit changes :-)
History
Date User Action Args
2017-10-03 09:40:04pitrousetnosy: + pitrou
messages: + msg303588
2014-10-25 20:44:27bwhmathersetfiles: + non-magic-waiters.patch

messages: + msg230017
2014-10-14 18:59:35ned.deilysetnosy: + bquinlan
2014-10-14 09:07:07bwhmathersetfiles: + change-docs-and-waiters.patch
2014-10-14 09:06:31bwhmathersetfiles: + change-docs.patch
2014-10-14 09:05:54bwhmathercreate