From b74f1ab11253aa03fda86256ebe2acbbebcdcede Mon Sep 17 00:00:00 2001 From: Joey Date: Wed, 12 Nov 2025 16:54:15 +0100 Subject: [PATCH] Fix premature cleanup in provide. (#1490) Co-authored-by: Joey Jurjens --- src/django_components/perfutil/provide.py | 14 ++++++++ tests/test_templatetags_provide.py | 43 +++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/src/django_components/perfutil/provide.py b/src/django_components/perfutil/provide.py index 94ad9df5..f695a4f1 100644 --- a/src/django_components/perfutil/provide.py +++ b/src/django_components/perfutil/provide.py @@ -93,12 +93,20 @@ provide_references: Dict[str, Set[str]] = defaultdict(set) # NOTE: We manually clean up the entries when components are garbage collected. component_provides: Dict[str, Dict[str, str]] = defaultdict(dict) +# Track which {% provide %} blocks are currently active (rendering). +# This prevents premature cache cleanup when components are garbage collected. +active_provides: Set[str] = set() + @contextmanager def managed_provide_cache(provide_id: str) -> Generator[None, None, None]: + # Mark this provide block as active + active_provides.add(provide_id) try: yield except Exception as e: + # Mark this provide block as no longer active + active_provides.discard(provide_id) # NOTE: In case of an error in within the `{% provide %}` block (e.g. when rendering a component), # we rely on the component finalizer to remove the references. # But we still want to call cleanup in case `{% provide %}` contained no components. @@ -106,11 +114,17 @@ def managed_provide_cache(provide_id: str) -> Generator[None, None, None]: # Forward the error raise e from None + # Mark this provide block as no longer active + active_provides.discard(provide_id) # Cleanup on success _cache_cleanup(provide_id) def _cache_cleanup(provide_id: str) -> None: + # Don't cleanup if the provide block is still active. + if provide_id in active_provides: + return + # Remove provided data from the cache, IF there are no more references to it. # A `{% provide %}` will have no reference if: # - It contains no components in its body diff --git a/tests/test_templatetags_provide.py b/tests/test_templatetags_provide.py index c525a5c3..0e646760 100644 --- a/tests/test_templatetags_provide.py +++ b/tests/test_templatetags_provide.py @@ -1297,3 +1297,46 @@ class TestProvideCache: Root.render() _assert_clear_cache() + + @djc_test(parametrize=PARAMETRIZE_CONTEXT_BEHAVIOR) + def test_provide_cache_not_cleaned_while_active(self, components_settings): + @register("injectee31") + class Injectee(Component): + template: types.django_html = """ +
{{ value }}
+ """ + + def get_template_data(self, args, kwargs, slots, context): + data = self.inject("my_provide") + return {"value": data.value} + + @register("root") + class Root(Component): + template: types.django_html = """ +
{{ content|safe }}
+ """ + + def get_template_data(self, args, kwargs, slots, context): + # Nested synchronous rendering triggers GC between components. + nested_template = Template( + """ + {% load component_tags %} + {% for i in "xxxxxxxxxx" %} + {% provide "my_provide" value="hello" %} + {% component "injectee31" %}{% endcomponent %} + {% component "injectee31" %}{% endcomponent %} + {% component "injectee31" %}{% endcomponent %} + {% endprovide %} + {% endfor %} + """ + ) + content = nested_template.render(Context({})) + return {"content": content} + + _assert_clear_cache() + + rendered = Root.render() + + # 10 iterations * 3 components = 30 occurrences + assert rendered.count(">hello") == 30 + _assert_clear_cache()