MagicStack / MagicStack/asyncpg
asyncpg __del__ methods call asyncio APIs directly, which isn't guaranteed to work and indeed, doesn't work
- Dominant language
- Python
- Stars
- 8.1k
- Forks
- 468
- PR merge metrics
- No merged PRs in 30d
Description
asyncpg has several `__del__` methods that call into asyncio APIs. But... you can't safely call into asyncio APIs from a `__del__` method, because `__del__` methods can be run in arbitrary threads, or at arbitrary moments when the loop's internal data structures are in an inconsistent state.
Discovered here: https://github.com/python-trio/trio-asyncio/issues/44
Looking at the traceback in that issue, I suspect that the reason this hasn't been noticed before is that the `__del__` method ultimately ends up calling `loop.call_soon(...)`. And if you do that from another thread, then with regular asyncio, things will mostly seem to work – it won't actually wake up the loop the way `call_soon_threadsafe` would, but if the loop is still running then it will eventually get run, and the default `call_soon` will not explode or otherwise notice if it's called from the wrong thread. But in that issue, someone's using asyncpg with an alternative asyncio loop that fails when `call_soon` is called from the wrong thread, and this breaks asyncpg.
I guess asyncpg should go through all its `__del__` methods and wrap them in a `loop.call_soon_threadsafe`?
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by inventorying asyncpg's __del__ methods and the loop.call_soon paths implicated by the linked Trio issue. Check each cleanup path against the thread-safety and loop-state concerns described there. Done means the affected cleanup behavior works with alternative asyncio loop implementations without unsafe direct calls.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- postgresql, python
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100