CabbageDevelopment / qasync

Python library for using asyncio in Qt-based applications.
BSD 2-Clause "Simplified" License
307 stars 43 forks source link

_IocpProactor._poll might need to account for _unregistered #90

Closed stat1c-void closed 9 months ago

stat1c-void commented 9 months ago

Hi!

I've been hitting a problem on Windows with IocpProactor not closing. Firstly, it seems to be solved in recent commits - when using master code the problem does not seem to appear anymore, thanks! It seems additional locking has solved it (#87).

I've asked my debugging question and described details on python forums: https://discuss.python.org/t/asyncio-iocpproactor-does-not-close/35206 Mind you, all my thoughts at the end seem to be completely wrong.

But I wonder if overridden _IocpProactor._poll is missing this code from upstream _poll?

        # Remove unregistered futures
        for ov in self._unregistered:
            self._cache.pop(ov.address, None)
        self._unregistered.clear()

https://github.com/python/cpython/blob/43a6e4fa4934fcc0cbd83f7f3dc1b23a5f79f24b/Lib/asyncio/windows_events.py#L825-L828

Because after some more debugging I expected that's the problem - items from _cache do not get evicted, they are unregistered, but hang in _cache. Unless _unregister does not get called at all with the overridden proactor? But I think it can be called from run_forever: https://github.com/python/cpython/blob/43a6e4fa4934fcc0cbd83f7f3dc1b23a5f79f24b/Lib/asyncio/windows_events.py#L332-L334

hosaka commented 9 months ago

Hi, thanks for bringing this up.

Reading the comment for cpython run_forever, it does suggest that the IocpProactor.close will wait forever, qasync also calls this method. So without popping futures from cache (even though they are marked as unregistered), the while self._cache in the IocpProactor.close will never close itself.

I this this should be added to the _poll override.

stat1c-void commented 9 months ago

Alright, should I do a PR?

hosaka commented 9 months ago

I'll submit it later on today, cheers

stat1c-void commented 9 months ago

Awesome, thanks!