MagicStack / asyncpg

A fast PostgreSQL Database Client Library for Python/asyncio.
Apache License 2.0
6.88k stars 399 forks source link

Cut BaseProtocol circular reference on close. #1049

Closed pteromys closed 11 months ago

pteromys commented 1 year ago

A bound method contains a reference to the instance it's bound to. Most of the time, bound methods are created lazily at access time by the descriptor protocol and discarded after calling. But saving a bound method as another attribute on the instance creates a long-lived cycle, here .timeout_callback.__self__, that needs to be explicitly broken if we don't want to wake up python's garbage collector to do it.

Without this change, the new assertion in the tests would fail, and pytest --pdb would show the bound methods _on_timeout and _on_waiter_completed at the end of p gc.get_referrers(protoref()).

Things I'm not too certain about:

pteromys commented 1 year ago

Oh, I like that idea. Done! Let's see if it breaks anything...

elprans commented 11 months ago

There turned out to be another cycle -- protocol -> transport -> protocol -- which doesn't seem to get broken on the transport side in a Proactor loop on Windows.

pteromys commented 11 months ago

Thanks for following up and fixing that! (Sorry, I should've said something right away when I saw the test failure on Windows and realized it was going to be over my head!)