Message413409
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 :( |
|
Date |
User |
Action |
Args |
2022-02-17 15:11:11 | asvetlov | set | recipients:
+ asvetlov, gvanrossum, njs, jab, alex.gronholm, chris.jerdonek, yselivanov, tinchester, iritkatriel, ajoino |
2022-02-17 15:11:11 | asvetlov | set | messageid: <1645110671.83.0.41667558618.issue46771@roundup.psfhosted.org> |
2022-02-17 15:11:11 | asvetlov | link | issue46771 messages |
2022-02-17 15:11:11 | asvetlov | create | |
|