search

python-trio / trio-asyncio

a re-implementation of the asyncio mainloop on top of Trio
Other
188 stars 37 forks source link

`trio.Event.clear()` is deprecated #61

Closed mehaase closed 4 years ago

mehaase commented 4 years ago

I am getting a deprecation warning:

.../python3.7/site-packages/trio_asyncio/base.py:718: TrioDeprecationWarning: trio.Event.clear is deprecated since Trio 0.12.0; use multiple Event objects or other synchronization primitives instead (https://github.com/python-trio/trio/issues/637)

The deprecation references https://github.com/python-trio/trio/issues/637 for more information. In short, Event.clear() is a foot gun that is prone to creating race conditions. The general recommendation is to create new events instead of clearing existing ones. It's not clear to me whether this recommendation would work in this circumstance:

https://github.com/python-trio/trio-asyncio/blob/4990c61421486c24095099a961c5b6a7f4110cdc/trio_asyncio/base.py#L718

The event is set as soon as its created, then cleared when control enter the asyncio main loop. This is an unsual way to use trio.Event, which makes me hesitant to say whether this is an easy to fix or not...

I also haven't checked if there are other uses of Event.clear() in the code.

parity3 commented 4 years ago

I'm getting this too. As a hack to remove the deprecation warning messages for the time being, I think it's pretty safe to put in the following code:

def clear_event(self):
    assert not self._stopped.statisticts().tasks_waiting, "unable to clear event due to other waiters"
    self._stopped = trio.Event()

At least then it will fail in an explicit way. I'm not planning on running more than 1 loop for the process lifetime of my app so I'm probably safe here.

pquentin commented 4 years ago

This also affects the pytest-trio tests: https://github.com/python-trio/pytest-trio/pull/91

pquentin commented 4 years ago

Fixed in https://github.com/python-trio/trio-asyncio/pull/66, 0.11.0 should be released soon.