[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.46%. Comparing base (1f102bb) to head (1306192).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #889      +/-   ##
==========================================
+ Coverage   95.43%   95.46%   +0.02%     
==========================================
  Files         134      134              
  Lines       26843    27020     +177     
==========================================
+ Hits        25618    25795     +177     
  Misses       1225     1225              

☔ 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?

[lieryan]

Ok, so I just tested this, the ConnectionCloser approach works to ensure that connections are closed as soon as the thread finishes, and if you also use weakref.finalize you can also close the connection in all threads when the thread local object becomes unreachable and gets garbage collected. With ConnectionCloser and weakref.finalize, we do not need to keep an explicit registry of the connections and can avoid most of the complexity of locking and maintaining a thread-safe registry. So it's an option for simpler, but more implicit and a bit "magical" implementation.

ConnectionCloser + finalize() implementation

diff --git a/rope/contrib/autoimport/sqlite.py b/rope/contrib/autoimport/sqlite.py
index 7ae8fb71..014ca6b8 100644
--- a/rope/contrib/autoimport/sqlite.py
+++ b/rope/contrib/autoimport/sqlite.py
@@ -9,6 +9,7 @@ import secrets
 import sqlite3
 import sys
 import warnings
+import weakref
 from collections import OrderedDict
 from concurrent.futures import Future, ProcessPoolExecutor, as_completed
 from datetime import datetime
@@ -143,10 +144,6 @@ class AutoImport:
                 DeprecationWarning,
             )
         self.thread_local = local()
-        self.connection = self.create_database_connection(
-            project=project,
-            memory=memory,
-        )
         self._setup_db()
         if observe:
             observer = resourceobserver.ResourceObserver(
@@ -186,10 +183,15 @@ class AutoImport:
             else:
                 project_hash = calculate_project_hash(project.ropefolder.real_path)
             return sqlite3.connect(
-                f"file:rope-{project_hash}:?mode=memory&cache=shared", uri=True
+                f"file:rope-{project_hash}:?mode=memory&cache=shared",
+                uri=True,
+                check_same_thread=False,
             )
         else:
-            return sqlite3.connect(project.ropefolder.pathlib / "autoimport.db")
+            return sqlite3.connect(
+                project.ropefolder.pathlib / "autoimport.db",
+                check_same_thread=False,
+            )
 
     @property
     def connection(self) -> sqlite3.Connection:
@@ -203,6 +205,11 @@ class AutoImport:
                 project=self.project,
                 memory=self.memory,
             )
+            self.thread_local.connection_finalizer = weakref.finalize(
+                self.thread_local,
+                self.thread_local.connection.close,
+            )
+
         return self.thread_local.connection
 
     @connection.setter
@@ -457,10 +464,17 @@ class AutoImport:
         self._del_if_exist(module)
         self.generate_modules_cache([module])
 
+    def close_all(self):
+        """Close autoimport for all threads."""
+        with contextlib.suppress(AttributeError):
+            # deleting the thread_local would run all the associated
+            # connection finalizers
+            del self.thread_local
+
     def close(self):
-        """Close the autoimport database."""
+        """Close autoimport for the current thread."""
         self.connection.commit()
-        self.connection.close()
+        self.thread_local.connection_finalizer()
 
     def get_name_locations(self, name):
         """Return a list of ``(resource, lineno)`` tuples."""

The problem is that just like any other approaches that wants to have a global close connection method, they are all going to still require check_same_thread=False because that's really the only way a connection can be closed from a different thread. There's just no working around that since you can't interrupt an uncooperating thread to tell it to run close() themselves.

Putting aside the issue of whether explicit or implicit registry, the more fundamental issue is that closing a connection from a different thread is inherently risky in sqlite3. If you close() a connection while another thread is in the middle of execute()-ing a command, you would segfault the entire Python interpreter. This is not a catchable exception. For example, this causes a Segmentation fault (tested on Linux):

segfault POC

import threading
import sqlite3
import time


def worker():
    sql = 'SELECT sleep(5)'
    print('worker: going to sleep')
    conn.execute(sql)

conn = sqlite3.connect(':memory:', check_same_thread=False)
conn.create_function('sleep', 1, time.sleep)

time.sleep(1)

t = threading.Thread(target=worker)
t.start()

print('main: closing connection')
conn.close()

check_same_thread=True prevents these kind of segfaults from happening, but requires that each thread to manage their own connection's lifetime so you can't have a global close.

I'm not quite sure there are any safe way to close() another threads' connections from the main thread without explicit cooperation from the workers to stop executing more sql commands, and that means that any safe global close implementation also necessarily has to be blocking since it has to wait for workers to finish whatever they're currently doing, somehow put a block to prevent the worker from running any more commands, and only then it can terminate the connection.

In Python, usually that means that either the workers would have to acquire a context manager before performing any operations, or the main thread has to arrange such that all the workers are already finished by the time the main thread is closing the connections.

[mcepl]

Following up on the connection-lifetime discussion. I pushed c4c0eca with a thread-to-connection registry that reaps connections only after their owner thread exits. close() now raises RopeError instead of closing a connection owned by another live thread. Callers must stop and join workers before closing the shared AutoImport, or have workers call close_thread_connection() themselves.

I also removed the __del__ close path, documented the shutdown contract, and added tests for reaping finished-worker connections and refusing close while a worker is alive. The AutoImport tests pass (78 passed).

Does this shutdown contract address the concern?