dabeaz / curio

Good Curio!
Other
4.01k stars 240 forks source link

UniversalEvent cant be set after wait time out #352

Closed vytasrgl closed 1 year ago

vytasrgl commented 2 years ago

Currently it seems if waiter for UniversalEvent times out the event becomes unusable and can not be set, and if there are multiple waiters those would not be informed either. The regular Event class works fine with the test case below.

For now i have no fix for it. Consider the following test case would fail with InvalidStateError: CANCELLED: <Future at 0x7fcefc671a30 state=cancelled> in curio/sync.py:100

    def test_uevent_get_wait_timeout(self, kernel):
        results = []
        async def event_setter(evt, seconds):
            results.append('sleep')
            await sleep(seconds)
            results.append('event_set')
            await evt.set()

        async def event_waiter(evt, seconds):
            try:
                results.append('wait_start')
                await timeout_after(seconds, evt.wait())
                results.append('wait_done')
                results.append(evt.is_set())
            except TaskTimeout:
                results.append('wait_timeout')
                results.append(evt.is_set())

        async def main():
            evt = UniversalEvent()
            t1 = await spawn(event_waiter, evt, 1)
            t2 = await spawn(event_setter, evt, 2)
            await t1.join()
            await t2.join()

        kernel.run(main())
        assert results == [
            'wait_start',
            'sleep',
            'wait_timeout',
            False,
            'event_set',
        ]

OS: Ubuntu 20.04 with Python3.8

dabeaz commented 2 years ago

Intriguing. I'll spend some time working on it!

dabeaz commented 2 years ago

Have committed an initial fix. Still need to add a few more tests.

vytasrgl commented 2 years ago

Thanks. The fix seem to solve issues i had, but i will test further and report if it doesn't work as expected.

vytasrgl commented 2 years ago

Even with 842a8c6 it seems there is issue with the locking. I am getting it while setting the event from regular thread.

Set changed size during iteration at:

/curio/sync.py", line 122, in set
curio/sync.py", line 113, in _unblock_waiters
    for fut in self._waiting:
dabeaz commented 2 years ago

I've made a few more changes to avoid this.

vytasrgl commented 2 years ago

After further testing seems working fine now. Thanks for prompt fix.

dabeaz commented 2 years ago

Thanks for submitting this. It revealed a number of bugs in the UniversalEvent implementation.