Cut BaseProtocol circular reference on close.#1049
Merged
elprans merged 4 commits intoMagicStack:masterfrom Oct 9, 2023
pteromys:callback-gc
Merged
Cut BaseProtocol circular reference on close.#1049elprans merged 4 commits intoMagicStack:masterfrom pteromys:callback-gc
elprans merged 4 commits intoMagicStack:masterfrom
pteromys:callback-gc
Conversation
elprans
reviewed
Jul 7, 2023
Member
elprans
left a comment
elprans
left a comment
There was a problem hiding this comment.
TBH, I'm not sure those variables are needed at all, because they always seem to be assigned to self._on_timeout and self._on_waiter_completed, so perhaps just remove them?
Contributor
Author
|
Oh, I like that idea. Done! Let's see if it breaks anything... |
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())`.
Member
|
There turned out to be another cycle -- |
elprans
approved these changes
Oct 9, 2023
Contributor
Author
|
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!) |
elprans
added a commit
that referenced
this pull request
Nov 5, 2023
Minor fixes and improvements. Improvements ============ * Python 3.12 and PostgreSQL 16 support (#1084) (by @elprans in deea86c) * Add support for tuple-format custom codecs on composite types (#1061) (by @elprans in 922fcd1) * Support `target_session_attrs` in URL format, add tests (#1073) (by @elprans in 7cb4e70) * Infinity numeric support (#1067) (by @krokoziabla in 0c3bf60 for #1020) * Add support for the `WHERE` clause in `copy_to` methods (#941) (by @kaylynn234 in b7ffab6) * Add query logging callbacks and context manager (#1043) (by @dcwatson in b2697ff) Fixes ===== * When prepared statements are disabled, avoid relying on them harder (#1065) (by @elprans in cbf64e1) * Handle environments with HOME set to a not-a-directory (#1063) (by @elprans in af922bc) * Fix handling of non-ASCII passwords (#1062) (by @elprans in 89d5bd0) * Disable JIT while doing type introspection (#1082) (by @elprans in f21ebf6) * Remove connection parameter caching in `Pool` (#1053) (by @ermakov-oleg in 4ddb039) * Switch to Python 3.12-style `wait_for` (#1086) (by @elprans in 4bdd8a7) * Update automatic PostGIS type conversion for Shapely 2.0 (#1085) (by @ChimneySwift in 8b45beb) * Use the `timeout` context manager in the connection path (#1087) (by @elprans in 313b2b2) * Small fix for documentation on using SSL in Connection (#995) (by @ScottFred in ccc7baf) * Use cleanup_ctx in pool usage doc (#878) (by @ir4y in 70c8bd8) * Close cursor portals once the iterator is exhausted (#1088) (by @elprans in ca9f03b) * Cut BaseProtocol circular reference on close. (#1049) (by @pteromys in 93a6f79) * Allow passing hosts as tuples to `connect()` (in addition to lists) (#1021) (by @lezram in d7faaff) Other ===== * Drop support for Python 3.7 (#1064) (by @bryanforbes in 87ab143)
Merged
elprans
added a commit
that referenced
this pull request
Nov 5, 2023
Minor fixes and improvements. Improvements ============ * Python 3.12 and PostgreSQL 16 support (#1084) (by @elprans in deea86c) * Add support for tuple-format custom codecs on composite types (#1061) (by @elprans in 922fcd1) * Support `target_session_attrs` in URL format, add tests (#1073) (by @elprans in 7cb4e70) * Infinity numeric support (#1067) (by @krokoziabla in 0c3bf60 for #1020) * Add support for the `WHERE` clause in `copy_to` methods (#941) (by @kaylynn234 in b7ffab6) * Add query logging callbacks and context manager (#1043) (by @dcwatson in b2697ff) Fixes ===== * When prepared statements are disabled, avoid relying on them harder (#1065) (by @elprans in cbf64e1) * Handle environments with HOME set to a not-a-directory (#1063) (by @elprans in af922bc) * Fix handling of non-ASCII passwords (#1062) (by @elprans in 89d5bd0) * Disable JIT while doing type introspection (#1082) (by @elprans in f21ebf6) * Remove connection parameter caching in `Pool` (#1053) (by @ermakov-oleg in 4ddb039) * Switch to Python 3.12-style `wait_for` (#1086) (by @elprans in 4bdd8a7) * Update automatic PostGIS type conversion for Shapely 2.0 (#1085) (by @ChimneySwift in 8b45beb) * Use the `timeout` context manager in the connection path (#1087) (by @elprans in 313b2b2) * Small fix for documentation on using SSL in Connection (#995) (by @ScottFred in ccc7baf) * Use cleanup_ctx in pool usage doc (#878) (by @ir4y in 70c8bd8) * Close cursor portals once the iterator is exhausted (#1088) (by @elprans in ca9f03b) * Cut BaseProtocol circular reference on close. (#1049) (by @pteromys in 93a6f79) * Allow passing hosts as tuples to `connect()` (in addition to lists) (#1021) (by @lezram in d7faaff) Other ===== * Drop support for Python 3.7 (#1064) (by @bryanforbes in 87ab143)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 --pdbwould show the bound methods_on_timeoutand_on_waiter_completedat the end ofp gc.get_referrers(protoref()).Things I'm not too certain about:
.timeout_callbackand.completed_callback—I empirically seem to have covered my use case, but that's far from actually understanding what gets called when