Issue 22630: `concurrent.futures.Future.set_running_or_notify_cancel` does not notify cancel
Created on 2014-10-14 09:05 by bwhmather, last changed 2022-04-11 14:58 by admin.
| 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 (4) | |||
|---|---|---|---|
| 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) * ![]() |
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 :-) |
|||
| msg341775 - (view) | Author: Brian Quinlan (bquinlan) * ![]() |
Date: 2019-05-07 16:26 | |
Ben, do you still think that your patch is relevant or shall we close this bug? |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:09 | admin | set | github: 66820 |
| 2019-05-07 16:26:55 | bquinlan | set | messages: + msg341775 |
| 2017-10-03 09:40:04 | pitrou | set | nosy:
+ pitrou messages: + msg303588 |
| 2014-10-25 20:44:27 | bwhmather | set | files:
+ non-magic-waiters.patch messages: + msg230017 |
| 2014-10-14 18:59:35 | ned.deily | set | nosy:
+ bquinlan |
| 2014-10-14 09:07:07 | bwhmather | set | files: + change-docs-and-waiters.patch |
| 2014-10-14 09:06:31 | bwhmather | set | files: + change-docs.patch |
| 2014-10-14 09:05:54 | bwhmather | create | |
