fix: close the connections the sqlite probes open - #401
Merged
Conversation
does_table_exist_in_db, does_view_exist_in_db, does_column_exist_in_db and null_absent_columns each open a connection and return without closing it, so a run holds one handle per probe. All four now close in a finally, which also covers the early return True path. does_column_exist_in_db additionally lacked the if db: guard its siblings have, so an unreadable database raised AttributeError rather than sqlite3.Error and killed the artifact instead of returning False. Measured by counting open file descriptors with gc disabled: 150 calls leaked 150 handles before and 0 after; 450 probe calls leaked 450 before and 0 after. Levelled from iLEAPP PR #1778 plus the null_absent_columns case it predates. 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.
Follow-up to the connection-leak fix in iLEAPP #1778, which fixed three helpers there. This brings the same fix to this core and covers a fourth function that #1778 predates.
What leaks
Each of
does_table_exist_in_db,does_view_exist_in_dbanddoes_column_exist_in_dbopens a connection throughopen_sqlite_db_readonly, returns a bool, and leaves the connection to the garbage collector.null_absent_columnsdoes the same. Artifacts probe the same database repeatedly, so this is one held handle per probe for the length of a run.does_column_exist_in_dbalso lacked theif db:guard its two siblings have. Whenopen_sqlite_db_readonlyreturnsNonefor an unreadable database it raisedAttributeError: 'NoneType' object has no attribute 'row_factory'— not asqlite3.Error, so the local handler missed it and the whole artifact died. One bad file costing an examiner every row.All four now close in a
finally, so the earlyreturn Truepath closes too, and the guard is consistent across the three.Measured, by counting open file descriptors
Garbage collection disabled so a leak cannot be hidden by collection:
null_absent_columnscallsdoes_*_exist_in_dbcallsOne leaked handle per call, in both cases, now zero. Behaviour is unchanged: the same query still comes back with the absent column reported as
NULLunder its own name.A
ResourceWarning-based test was tried first and is not adequate: the warning is raised during garbage collection, where CPython prints "Exception ignored" instead of propagating, so the test reported "no leak" against the unfixed code as well. Descriptor counting was verified to fail on the unfixed version before being trusted.Levelled across all five cores so they stay identical.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com