Andrew Svetlov <andrew.svet...@gmail.com> added the comment: The discussion is hot, I see several interleaved threads.
Let me put my answers on all of them in order of appearance. 1. quattro cancellation scopes are implemented after async-timeout. As the author of async-timeout I am happy to know it. The module is pretty small (as async-timeout itself). I'd like to concentrate on the differences between async-timeout and quattro. a) quattro has `fail_after()` / `fail_at()` context managers, that are similar to async-timeout's `timeout()` / `timeout_at()` The first function schedules timeout as a *relative* delay, the second uses *absolut monotonic* time. So far so good. `timeout()` better points on 'what happens' I believe (TimeoutError is raised). `fail_after` is not that bad at all but `timeout()` is easier I think. `timeout_at()` and `fail_at()` are minic to `loop.call_at()`, the prefix is perfect. Regarding `loop.call_later()` I found that `timeout_later()` is too long name (plus `timeout()` context manager appeared in 2015 as a part of aiohttp). My opinion is biased, sure. b) quattro has `move_on_after()` and `move_on_at()` context managers that doesn't raise TimeoutError but set a flag after certain period. asyncio has lower level primitives than trio (`loop.call_later()` and `loop.call_at()`). Not sure that `move_on_*` should be added, they are low-level building blocks. On the other hand, `move_on_after()` requires less code lines than `call_later()`. If we decide 'yes' I suggest changing naming: I have no idea what is *moved* without reading the documentation. c) `with fail_after()` vs `async with timeout()`. The first async-timeout version used `with timeout()` notation. I've changed it to async counterpart after many questions from newbies: "Why is sync context manages used in async code?". Questions arose when asyncio was not wide-spread as today. People made their first baby steps those days, I provided a dozen of asyncio offline courses and many conference talks. Thus, please consider `async with ...` design as a user feedback reaction. As a side effect, it prohibits `timeout()` usage in non-async functions (which is awkward at least). Regarding async context manager performance, I think it is good enough for timeout-related things. I didn't experience any problem with it. Moreover, async fast-path (async function is called and it returns a value without suspension on awaiting) can be optimized on Python level to make it as fast as a regular python function call, sure. It is not trivial and might require adding a new opcode (combine CALL + GET_AWAITABLE) but this optimization is out of the issue scope. d) `cm.deadline += 0.5` vs `cm.shift(0.5)` is a question of taste. asyncio-timeout design motivation was "don't do complex things in property setter" but I can live with mutable `cm.deadline` attribute, sure e) cancellation stack and `current_effective_deadline` -- I'm with Guido, let's not add yet another stack. It can be an interesting debug feature but I doubt if it is useful in production code. Also, the performance cost is not zero. Merging and slicing stack tuple on any timeout context enter/exit is not free. The implementation can be switched to a linked list but still, do we really need it? 2. Alex Grönholm, asyncio supports custom task instances without overriding the entire task factory. You should provide a custom method for custom task creatuon, that's it. `asyncio.all_tasks()` / `asyncio.current_task()` support is provided by '_register_task()', '_unregister_task()', '_enter_task()', and '_leave_task()' calls. These methods are part of non-user-faced public API, they are intentionally enumerated by `asyncio.__all__`. These methods are mentioned by changelog only, sorry. A pull request for documenting them in asyncio low-level section is welcome! 3. The race condition between two `.cancel()` calls performed by the same loop iteration. Sure, the race exists. Before TaskGroup landing, the last `.cancel()` wins. After the change, the first `.cancel()` wins and all subsequent `.cancel()` calls made on the same event loop iteration are rejected with returning `False`. I believe, the changed behavior is more consistent (and close to `Future.cancel()` design). Assume, we have two scheduled close but different timeouts for the same tasks. Both are reached at the next event loop iteration (see the timeline below): prev-iteration timeout-a timeout-b current-iteration | | | | >---+--------------+-----------+--------------+--------------> future I prepared https://github.com/aio-libs/async-timeout/pull/295 to handle it (not merged yet because the next Python alpha release it required; I've tested it against the latest CPython main branch manually). Sorry for polluting source code by `sys.version_info` checks, the library is supposed to work with Python 3.7+. async-timeout handles the race as follows: https://github.com/aio-libs/async-timeout/blob/master/async_timeout/__init__.py#L209-L239 a) timeout-a and timeout-b TimeHandle's are scheduled for execution on the current iteration and executed. b) Both context managers set their own state to 'TIMEOUT'. c) timeout-a cancels the current task, timeout-b calls the cancel but it is skipped and rejected because timeout-a called `task.cancel()` earlier. d) The task is cancelled, async stack unwinds. e) The inner context manager converts CancelledError to TimeoutError and raises it. f) The outer context manager does nothing but bubbles-up TimeoutError from the inner context manager. Doesn't matter what context manager (timeout-a or timeout-b) is inner and what is outer; it the timeout occurs the TimeoutError is propagated from the inner code up along the stack *unless* some code swallows it intentionally. I don't think that we should prohibit swallowing. Another edge case is the task explicit cancellation that is scheduled on the same event loop iteration as timeout occurrence: prev-iteration (explicit .cancel() called) timeout current-iteration | | | >-----------------+----------------------------+-------------+----------> >future a) `loop.call_later()` registered callback (`_on_timeout`) is called first. Its `task.cancel()` is rejected (`False` returned) because the first cancellation was explicitly requested on the previous iteration. b) the task wakes up to handle `.cancel()` call from the prev iteration c) stack unwinds with CancelledError, it is not converted to TimeoutError because CancelledError has no required message. That's fine, because `.cancel()` call from the previous iteration is processed, not timeout itself. d) `.expired` property is `True` though because timeout is reached technically; the wall-clock time is greater than the deadline. 4. Tin Tvrtković, I don't think that a separate field for 'nonce' is needed for the proper cancellation. It adds more complexity; I have no idea what to do with the nonce field if the CancelledError comes not from timeout context manager but is rased by other logic. As I demonstrated above, using cancellation message as nonce work perfectly fine. P.S. It is a long letter, sorry. Please don't hesitate to discuss it, feel free to ask a question if some of my words are not clear. English is not my mother language :( ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <https://bugs.python.org/issue46771> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com