[issue22630] `concurrent.futures.Future.set_running_or_notify_cancel` does not notify cancel
New submission from Ben Mather: 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. -- assignee: docs@python components: Documentation, Library (Lib) files: change-callbacks.patch keywords: patch messages: 229282 nosy: bwhmather, docs@python priority: normal severity: normal status: open title: `concurrent.futures.Future.set_running_or_notify_cancel` does not notify cancel versions: Python 3.2, Python 3.3, Python 3.4, Python 3.5 Added file: http://bugs.python.org/file36908/change-callbacks.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22630 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22630] `concurrent.futures.Future.set_running_or_notify_cancel` does not notify cancel
Changes by Ben Mather bwhmat...@bwhmather.com: Added file: http://bugs.python.org/file36909/change-docs.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22630 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22630] `concurrent.futures.Future.set_running_or_notify_cancel` does not notify cancel
Changes by Ben Mather bwhmat...@bwhmather.com: Added file: http://bugs.python.org/file36910/change-docs-and-waiters.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22630 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22729] `wait` and `as_completed` depend on private api
New submission from Ben Mather: Adds `remove_done_callback` to `Future` object, removes `_waiters` attribute, and re-implements `wait` and `as_completed` using callbacks. This makes it possible to extend or replace `Future` without having to mimic its private `_waiters` interface. Currently waiters and callbacks are triggered at different points after a cancel (waiters in `set_condition_and_notify_cancel` and callbacks in `cancel`.) This is a problem as it means that implementing `wait` and `as_completed` using `add_done_callback` will result in a behaviour change unless the behaviour of `add_done_callback` is changed instead. I don't believe the current behaviour is as documented anyway so I'm not sure if this is a problem. See issue 22630. I am a little uncertain about the O(n) implementation of `remove_done_callback` but can't imagine a situation where it would be a problem. -- components: Library (Lib) files: non-magic-waiters.patch keywords: patch messages: 230016 nosy: bwhmather priority: normal severity: normal status: open title: `wait` and `as_completed` depend on private api type: behavior versions: Python 3.4, Python 3.5, Python 3.6 Added file: http://bugs.python.org/file37016/non-magic-waiters.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22729 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22630] `concurrent.futures.Future.set_running_or_notify_cancel` does not notify cancel
Ben Mather added the comment: Have uploaded patch to re-implement `wait` and `as_completed` using callbacks. See issue 22729. Sorry for sitting on it for so long. -- Added file: http://bugs.python.org/file37017/non-magic-waiters.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22630 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22729] `wait` and `as_completed` depend on private api
Ben Mather added the comment: Sorry for slow response. I completely missed your reply. I was working on a project that used a concurrent.futures thread pool but also needed to listen to responses from a chip-and-pin card reader. Payment sessions operated in three phases: - check with card reader that payment is possible - save payment to database as if it has happened (always best to undercharge in the case of an error) - ask the payment provider to commit the payment Up until confirmation is received, it is entirely possible to cancel the session. Once confirmation is received the session starts trying to commit and you can only really wait for it to finish and then roll back (or break everything). This maps pretty well to the interface of future (though with very different plumbing and with work happening before cancellation stops working) and it made sense to try to write it so that it could interoperate with futures representing tasks running on the thread pool. I tried to add a method which returned a plain concurrent.futures.Future object but couldn't find a way to hook into the cancel method without introducing loads of race conditions. Really it just seemed wrong that I had an object that quacked like a duck but wasn't a duck because real ducks communicated over a hidden side channel. The card reader library is open source and mostly functional. The class is in: https://github.com/bwhmather/python-payment-terminal/blob/develop/payment_terminal/drivers/bbs/payment_session.py I'm sure there are other places where this sort of interoperability would be useful. -- ___ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue22729> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22729] concurrent.futures `wait` and `as_completed` depend on private api
Ben Mather <bwhmat...@bwhmather.com> added the comment: The patch is indeed a little outdated, but I would be happy to pick it and get it working again. First we need a resolution to #22630 though. Currently calling `cancel` will invoke callbacks, but waiters won't be triggered until `set_running_or_notify_cancel` is called. If both are implemented using the same mechanism, then both will need to be cancelled at the same time. This means that the behaviour of one the other will need to be changed in a backwards incompatible way. Does anyone have a preference for where we should send notification of cancellation? Is the breakage significant enough to kill this patch? -- ___ Python tracker <rep...@bugs.python.org> <https://bugs.python.org/issue22729> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue22729] concurrent.futures `wait` and `as_completed` depend on private api
Ben Mather <bwhmat...@bwhmather.com> added the comment: @Nathaniel RE `remove_done_callback`: This was added as an optimisation to allow the `as_completed` iterator to remove callbacks for future that it has visited instead of having to store a separate set of visited futures and ignore them when their callbacks are triggered. This matches how the private waiters mechanism works. It might be worth submitting the core of this change without `remove_done_callback` and then discussing it later as a separate optimisation. I will experiment. -- ___ Python tracker <rep...@bugs.python.org> <https://bugs.python.org/issue22729> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
