MagicStack / MagicStack/asyncpg

asyncpg __del__ methods call asyncio APIs directly, which isn't guaranteed to work and indeed, doesn't work

Open
#376 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

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

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.