Repository navigation
Close connections opened by other threads in AutoImport.close() - #888
cristianchiriac wants to merge 3 commits into
Conversation
AutoImport opens one SQLite connection per thread, but close() only closed the one of the calling thread, so connections made by worker threads were left open (ResourceWarning: unclosed database). Keep track of every thread's connection and close them all in close(). Connections of threads that have finished are closed when a new one is opened, so the list doesn't grow in long running processes. Fixes python-rope#886
…e-thread-connections # Conflicts: # CHANGELOG.md
|
Thanks for noting the overlap with #889. Both PRs address the same cross-thread SQLite connection leak. My preference is to continue with #889's thread-safe connection registry and explicit worker cleanup API, while preserving the useful lifetime tests from this PR. Could you review that implementation against the cases handled here, especially connections owned by threads that have exited? Please also flag any differences in cleanup timing or connection retention. Concurrent connection creation and AutoImport.close() deserve explicit coverage before we choose the final version. There is no need to close this PR yet. Let's consolidate the tests and confirm the replacement before doing that. |
|
Reviewed #889 against the cases here: #889 (comment). It handles close() across threads and concurrent creation. It does not close connections of threads that have exited (they stay open until close()); the failing test is in that comment. I'll close this PR once #889 has the tests. |
…e-thread-connections Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Description
AutoImportopens one SQLite connection per thread, butclose()only closed the connection of the thread calling it. Connections opened by worker threads stayed open until they were garbage collected, which shows up asResourceWarning: unclosed databaseon Python 3.13+.This keeps track of each thread's connection and closes all of them in
close(). Connections of threads that have already finished are closed when a new thread opens its connection, so the list doesn't keep growing in long running processes.To be able to close them from another thread, connections are now created with
check_same_thread=False. Each thread still only uses its own connection.Note: most of the
unclosed databasewarnings in the test suite come from tests that create anAutoImportand never callclose(), so this change doesn't remove those.Fixes #886
Checklist (delete if not relevant):