Switch StaticFilesPanel to use ContextVar. (#1801)

The StaticFilesPanel thread collector was not closing connections
to the database. This approach allows those connections to be closed
while still collecting context across threads.

The test case is to hit an admin login screen with

    ab -n 200 http://localhost:8000/admin/login/

As far as I can tell, all the static files are collected properly and
connections don't stay open.

Author: tim-schilling

debug_toolbar/panels/staticfiles.py

@@ -1,3 +1,5 @@
+import contextlib
+from contextvars import ContextVar
 from os.path import join, normpath
 
 from django.conf import settings
@@ -7,12 +9,6 @@
 from django.utils.translation import gettext_lazy as _, ngettext
 
 from debug_toolbar import panels
-from debug_toolbar.utils import ThreadCollector
-
-try:
-    import threading
-except ImportError:
-    threading = None
 
 
 class StaticFile:
@@ -33,15 +29,8 @@ def url(self):
         return storage.staticfiles_storage.url(self.path)
 
 
-class FileCollector(ThreadCollector):
-    def collect(self, path, thread=None):
-        # handle the case of {% static "admin/" %}
-        if path.endswith("/"):
-            return
-        super().collect(StaticFile(path), thread)
-
-
-collector = FileCollector()
+# This will collect the StaticFile instances across threads.
+used_static_files = ContextVar("djdt_static_used_static_files")
 
 
 class DebugConfiguredStorage(LazyObject):
@@ -65,15 +54,16 @@ def _setup(self):
             configured_storage_cls = get_storage_class(settings.STATICFILES_STORAGE)
 
         class DebugStaticFilesStorage(configured_storage_cls):
-            def __init__(self, collector, *args, **kwargs):
-                super().__init__(*args, **kwargs)
-                self.collector = collector
-
             def url(self, path):
-                self.collector.collect(path)
+                with contextlib.suppress(LookupError):
+                    # For LookupError:
+                    # The ContextVar wasn't set yet. Since the toolbar wasn't properly
+                    # configured to handle this request, we don't need to capture
+                    # the static file.
+                    used_static_files.get().append(StaticFile(path))
                 return super().url(path)
 
-        self._wrapped = DebugStaticFilesStorage(collector)
+        self._wrapped = DebugStaticFilesStorage()
 
 
 _original_storage = storage.staticfiles_storage
@@ -97,7 +87,7 @@ def title(self):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.num_found = 0
-        self._paths = {}
+        self.used_paths = []
 
     def enable_instrumentation(self):
         storage.staticfiles_storage = DebugConfiguredStorage()
@@ -120,18 +110,22 @@ def nav_subtitle(self):
         ) % {"num_used": num_used}
 
     def process_request(self, request):
-        collector.clear_collection()
-        return super().process_request(request)
+        reset_token = used_static_files.set([])
+        response = super().process_request(request)
+        # Make a copy of the used paths so that when the
+        # ContextVar is reset, our panel still has the data.
+        self.used_paths = used_static_files.get().copy()
+        # Reset the ContextVar to be empty again, removing the reference
+        # to the list of used files.
+        used_static_files.reset(reset_token)
+        return response
 
     def generate_stats(self, request, response):
-        used_paths = collector.get_collection()
-        self._paths[threading.current_thread()] = used_paths
-
         self.record_stats(
             {
                 "num_found": self.num_found,
-                "num_used": len(used_paths),
-                "staticfiles": used_paths,
+                "num_used": len(self.used_paths),
+                "staticfiles": self.used_paths,
                 "staticfiles_apps": self.get_staticfiles_apps(),
                 "staticfiles_dirs": self.get_staticfiles_dirs(),
                 "staticfiles_finders": self.get_staticfiles_finders(),

debug_toolbar/utils.py

@@ -14,12 +14,6 @@
 
 from debug_toolbar import _stubs as stubs, settings as dt_settings
 
-try:
-    import threading
-except ImportError:
-    threading = None
-
-
 _local_data = Local()
 
 
@@ -357,33 +351,3 @@ def get_stack_trace(*, skip=0):
 def clear_stack_trace_caches():
     if hasattr(_local_data, "stack_trace_recorder"):
         del _local_data.stack_trace_recorder
-
-
-class ThreadCollector:
-    def __init__(self):
-        if threading is None:
-            raise NotImplementedError(
-                "threading module is not available, "
-                "this panel cannot be used without it"
-            )
-        self.collections = {}  # a dictionary that maps threads to collections
-
-    def get_collection(self, thread=None):
-        """
-        Returns a list of collected items for the provided thread, of if none
-        is provided, returns a list for the current thread.
-        """
-        if thread is None:
-            thread = threading.current_thread()
-        if thread not in self.collections:
-            self.collections[thread] = []
-        return self.collections[thread]
-
-    def clear_collection(self, thread=None):
-        if thread is None:
-            thread = threading.current_thread()
-        if thread in self.collections:
-            del self.collections[thread]
-
-    def collect(self, item, thread=None):
-        self.get_collection(thread).append(item)

docs/changes.rst

@@ -9,6 +9,8 @@ Pending
   <https://beta.ruff.rs/>`__.
 * Converted cookie keys to lowercase. Fixed the ``samesite`` argument to
   ``djdt.cookie.set``.
+* Converted ``StaticFilesPanel`` to no longer use a thread collector. Instead,
+  it collects the used static files in a ``ContextVar``.
 
 4.1.0 (2023-05-15)
 ------------------