fix: close the connection null_absent_columns opens - #1916
Merged
Conversation
It opens a connection through open_sqlite_db_readonly to compile the query and returns without closing it. Artifacts call it once per query, so this is one held handle per query for the length of a run rather than a one-off, and it is now wired into 534 call sites here. Measured by counting open file descriptors with gc disabled: 150 calls leaked 150 handles before, 0 after. Behaviour unchanged. Companion to #1778, which fixed the same class in the three does_*_exist_in_db helpers and predates this function. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
null_absent_columnsopens a connection throughopen_sqlite_db_readonlyto compile the query withEXPLAIN, and returns without closing it. Artifacts call it once per query, and it is now wired into 534 call sites in this repo, so this is one held handle per query for the length of a run rather than a one-off.Same class as the three
does_*_exist_in_dbhelpers fixed in #1778, which this function postdates.Measured, by counting open file descriptors
Garbage collection disabled so a leak cannot be hidden by collection:
null_absent_columnscallsBehaviour is unchanged: the same query still returns with the absent column reported as
NULLunder its own name.A
ResourceWarning-based test was tried first and is not adequate. That warning is raised during garbage collection, where CPython prints "Exception ignored" rather than propagating, so the test reported "no leak" against the unfixed code too. Descriptor counting was confirmed to fail on the unfixed version before being trusted.Levelled to the other four cores: ALEAPP #1081, DLEAPP #61, RLEAPP #401, VLEAPP #118. Those also carry the #1778 helper fixes, which had not reached them.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com