[mcepl]

AutoImport dynamically allocates per-thread SQLite connections using threading.local(). However, AutoImport.close() only closed the connection belonging to the calling thread, orphaning any connections created by worker threads and triggering ResourceWarning on Python 3.12+.

Keep a thread-safe registry of all active connections on the AutoImport instance, create connections with check_same_thread=False, and ensure AutoImport.close() commits and closes all registered connections. Also expose close_thread_connection() for worker threads and implement context manager protocol support on AutoImport.

Fixes: #886

Checklist (delete if not relevant):

• I have added tests that prove my fix is effective or that my feature works
• I have updated CHANGELOG.md
• I have made corresponding changes to user documentation for new features
• I have made corresponding changes to library documentation for API changes

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.43%. Comparing base (2a29095) to head (1ea2f26).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #889      +/-   ##
==========================================
+ Coverage   95.41%   95.43%   +0.02%     
==========================================
  Files         134      134              
  Lines       26802    26929     +127     
==========================================
+ Hits        25574    25701     +127     
  Misses       1228     1228              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[cristianchiriac]

I ran the lifetime tests from #888 against this branch (5756c12), plus two extra cases mcepl asked about. Results:

• close() closes connections created by other threads: handled. test_close_closes_connections_of_other_threads passes.
• Concurrent connection creation, then close(): handled. 8 threads created connections at the same time (behind a threading.Barrier), all 8 were registered and all were closed by close().
• Connections owned by threads that have exited: not handled. They stay open and registered until close() (or until the worker calls close_thread_connection() itself). test_connections_of_finished_threads_are_closed fails with DID NOT RAISE ProgrammingError:

def test_connections_of_finished_threads_are_closed(project: Project):
    with closing(AutoImport(project, memory=True)) as autoimport:
        with ThreadPoolExecutor(1) as tp:
            worker_connection = tp.submit(lambda: autoimport.connection).result()

        # The next new connection closes the ones left by finished threads
        with ThreadPoolExecutor(1) as tp:
            tp.submit(lambda: autoimport.connection).result()

        with pytest.raises(sqlite3.ProgrammingError, match="closed database"):
            worker_connection.execute("SELECT 1")

The retention also adds up: after 20 short-lived threads, each touching autoimport.connection once, len(autoimport._connections) is 21. Before this change, those connections were garbage-collected when their thread ended, with a ResourceWarning. Now the registry keeps them alive for the whole lifetime of the AutoImport, so a long-lived instance used from per-request threads keeps collecting open connections unless every worker calls close_thread_connection().

#888 handles this by keying the registry by Thread. Whenever a new connection is registered, it closes and drops the entries whose thread is no longer is_alive(). Doing the same here would mean switching _connections from a set to a Dict[Thread, sqlite3.Connection] and doing that sweep in _register_connection, under the lock that is already held there.

One minor point: the connection setter drops the old connection from the registry but does not close it, so it stays open and is no longer tracked. Master and #888 have the same behaviour, so this is not a regression.

Tested on Python 3.14, Windows.

Once the exited-thread test (or an equivalent) is in this PR, I'm happy to close #888.

[lieryan]

I haven't tested this, but if avoiding the warning is all that's needed, I think we can simplify the implementation of this connection closing mechanism.

If we put a ConnectionCloser object into the thread local, then when the thread is finished, it will garbage collect the thread_local, which would garbage collect the thread closer, which can close the sqlite connection:

class ConnectionCloser:
    def __init__(self, connection):
        self.connection = connection
    def __del__(self):
        self.connection.close()

@property
def connection(self):
    """
    Creates a new connection if called from a new thread.

    This makes sure AutoImport can be shared across threads.
    """
    if not hasattr(self.thread_local, "connection"):
        self.thread_local.connection = self.create_database_connection(
            project=self.project,
            memory=self.memory,
        )
        self.thread_local.closer = ConnectionCloser(self.thread_local.connection)
        
    return self.thread_local.connection

The connection closer should run on the same thread as the connection object, so it would not need to bypass check_same_thread.

And I think a global close() method can also be implemented by disposing the thread local object, which would garbage collect all the closers and close all the connections? But if this garbage collection happens on the thread calling close(), it'll probably cause issues with closing the connection. Hmm..

I'd really prefer not to have to bypass the check_same_thread flag if possible. Force closing connections created by one thread from a different thread doesn't really sound like the correct pattern of normal usage.

Ideally, if worker threads don't want their connection objects to stay alive till the end of their thread, it should be that thread's responsibility to enter a context manager to manage the lifetime of their connection. My preference for AutoImport's concurrency model when the thread_local was originally implemented was that each worker threads should usually be creating their own instance of AutoImport. The use of thread local here means that each threads can have their own connection, but then they weren't required to manage the lifetime of their connection.

I'll need to let this simmer in my head for a bit.

[mcepl]

I agree that each worker owning its own AutoImport is a cleaner usage model. But since the class already uses thread-local connections, I think AutoImport.close() should still close every connection it has created.

I am not sure a ConnectionCloser.__del__ gives us that guarantee. It may help clean up a connection when its thread exits, but it does not give AutoImport.close() a reliable way to find the other connections. I would prefer to keep the registry and check_same_thread=False, then close connections belonging to exited threads when registering a new one. That would address the accumulation from short-lived workers, while close_thread_connection() lets a worker clean up promptly.

Would that approach address your concern, or would you prefer a different ownership model for these connections?