Skip to content

Commit

Permalink
Switch StaticFilesPanel to use ContextVar. (#1801)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tim-schilling committed Jul 3, 2023
1 parent 58f5ae3 commit 47d4eed
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 65 deletions.
52 changes: 23 additions & 29 deletions debug_toolbar/panels/staticfiles.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import contextlib
from contextvars import ContextVar
from os.path import join, normpath

from django.conf import settings
Expand All @@ -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:
Expand All @@ -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):
Expand All @@ -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
Expand All @@ -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()
Expand All @@ -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(),
Expand Down
36 changes: 0 additions & 36 deletions debug_toolbar/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()


Expand Down Expand Up @@ -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)
2 changes: 2 additions & 0 deletions docs/changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)
------------------
Expand Down

0 comments on commit 47d4eed

Please sign in to comment.