Fix race conditions, support Python 3.11

[asvetlov]

No description provided.

[asvetlov]

Python 3.11 tests fail, not released yet Python 3.11.0a6+ required

[Dreamsorcerer]

If this fixes the same race conditions we were looking at before, can you put the tests back in from that discussion? Seems you reverted them for some reason: ab04eb5

[asvetlov]

Thanks, I lost these tests.
Not sure if 1ms delay produces stable CI. Let me port tests and see how they work

[Dreamsorcerer]

Thanks, I lost these tests. Not sure if 1ms delay produces stable CI. Let me port tests and see how they work

It worked 100% of the times we tried it so far. If the last assert for call_order fails, then the timing went wrong. If it fails anywhere else, then the race condition is not fixed.

[Dreamsorcerer]

I think this will still fail the second test. If the timeout cancellation is followed by the explicit cancellation, then the explicit cancellation is ignored. Without fixing this, it would require a user to do something awkward everytime they cancel a task, like:

while not task.cancel():
    await asyncio.sleep(0)

I think we'll still need https://bugs.python.org/issue45098
With the new ExceptionGroup, I think my suggestion should change to raising multiple CancelledErrors. Then we can replace the timeout CancelledError with a TimeoutError and reraise any other CancelledErrors.

[asvetlov]

@Dreamsorcerer you may be interested in CPython discussion about asyncio timeouts design: https://bugs.python.org/issue46771?@ok_message=msg%20413603%20created%0Aissue%2046771%20message_count%2C%20messages%20edited%20ok&@template=item

Please feel free to join us.
I suggest suspending asyncio-timeout upgrading until the asyncio timeout has landed.

[Dreamsorcerer]

Following what Guido was saying, I think this needs to check to check the cancelled count, maybe something like:

Suggested change

	skip = False
	if sys.version_info >= (3, 9):
	# Analyse msg
	assert exc_val is not None
	if not exc_val.args or exc_val.args[0] != id(self):
	skip = True
	if not skip:
	if sys.version_info >= (3, 11):
	asyncio.current_task().uncancel()
	raise asyncio.TimeoutError
	self._timeout_handler = None
	if sys.version_info >= (3, 11):
	if asyncio.current_task().uncancel() == 0:
	self._timeout_handler = None
	raise asyncio.TimeoutError()
	else:
	raise asyncio.TimeoutError()

I think this is all that's needed, as the == _State.TIMEOUT tells us that we initiated a cancel, so there's no need for the cancel message anymore. Would be good to test this out against those previous tests before wrapping up the cpython discussion.

[Dreamsorcerer]

Suggested change

	assert timeout_cm.expired
	assert timeout_cm.expired
	async def race_condition(offset: float = 0) -> List[str]:
	"""Common code for below race condition tests."""
	async def test_task(deadline: float, loop: asyncio.AbstractEventLoop) -> None:
	# We need the internal Timeout class to specify the deadline (not delay).
	# This is needed to create the precise timing to reproduce the race condition.
	with pytest.warns(DeprecationWarning):
	with Timeout(deadline, loop):
	await asyncio.sleep(10)
	call_order: List[str] = []
	f_exit = log_func(Timeout._do_exit, "exit", call_order)
	Timeout._do_exit = f_exit # type: ignore[assignment]
	f_timeout = log_func(Timeout._on_timeout, "timeout", call_order)
	Timeout._on_timeout = f_timeout # type: ignore[assignment]
	loop = asyncio.get_running_loop()
	deadline = loop.time() + 1
	t = asyncio.create_task(test_task(deadline, loop))
	loop.call_at(deadline + offset, log_func(t.cancel, "cancel", call_order))
	# If we get a TimeoutError, then the code is broken.
	with pytest.raises(asyncio.CancelledError):
	await t
	return call_order
	@pytest.mark.skipif(sys.version_info < (3, 11), reason="Only fixed in 3.11+")
	@pytest.mark.asyncio
	async def test_race_condition_cancel_before() -> None:
	"""Test race condition when cancelling before timeout.
	If cancel happens immediately before the timeout, then
	the timeout may overrule the cancellation, making it
	impossible to cancel some tasks.
	"""
	call_order = await race_condition()
	# This test is very timing dependant, so we check the order that calls
	# happened to be sure the test itself ran correctly.
	assert call_order == ["cancel", "timeout", "exit"]
	@pytest.mark.skipif(sys.version_info < (3, 11), reason="Only fixed 3.11+.")
	@pytest.mark.asyncio
	async def test_race_condition_cancel_after() -> None:
	"""Test race condition when cancelling after timeout.
	Similarly to the previous test, if a cancel happens
	immediately after the timeout (but before the __exit__),
	then the explicit cancel can get overruled again.
	"""
	call_order = await race_condition(0.000001)
	# This test is very timing dependant, so we check the order that calls
	# happened to be sure the test itself ran correctly.
	assert call_order == ["timeout", "cancel", "exit"]

[asvetlov]

Version 5.0+ works exactly as asyncio built-in does, nothing to do here.