From ecb59cc6579402b68ddfd4499bf30edacf5963be Mon Sep 17 00:00:00 2001 From: Alex Hill Date: Wed, 9 Mar 2016 11:35:39 +0800 Subject: [PATCH] Fixed #26306 -- Fixed memory leak in cached template loader. --- django/template/backends/django.py | 17 +++++++++-- django/template/loaders/cached.py | 27 +++++++++++++++-- docs/releases/1.9.5.txt | 2 ++ tests/template_tests/test_loaders.py | 43 +++++++++++++++++++++++++--- 4 files changed, 79 insertions(+), 10 deletions(-) diff --git a/django/template/backends/django.py b/django/template/backends/django.py index cc795aae1c..afe1a7cd23 100644 --- a/django/template/backends/django.py +++ b/django/template/backends/django.py @@ -68,13 +68,24 @@ class Template(object): reraise(exc, self.backend) +def copy_exception(exc, backend=None): + """ + Create a new TemplateDoesNotExist. Preserve its declared attributes and + template debug data but discard __traceback__, __context__, and __cause__ + to make this object suitable for keeping around (in a cache, for example). + """ + backend = backend or exc.backend + new = exc.__class__(*exc.args, tried=exc.tried, backend=backend, chain=exc.chain) + if hasattr(exc, 'template_debug'): + new.template_debug = exc.template_debug + return new + + def reraise(exc, backend): """ Reraise TemplateDoesNotExist while maintaining template debug information. """ - new = exc.__class__(*exc.args, tried=exc.tried, backend=backend) - if hasattr(exc, 'template_debug'): - new.template_debug = exc.template_debug + new = copy_exception(exc, backend) six.reraise(exc.__class__, new, sys.exc_info()[2]) diff --git a/django/template/loaders/cached.py b/django/template/loaders/cached.py index 5bf5c10586..70c55feb51 100644 --- a/django/template/loaders/cached.py +++ b/django/template/loaders/cached.py @@ -7,6 +7,7 @@ import hashlib import warnings from django.template import Origin, Template, TemplateDoesNotExist +from django.template.backends.django import copy_exception from django.utils.deprecation import RemovedInDjango20Warning from django.utils.encoding import force_bytes from django.utils.inspect import func_supports_parameter @@ -27,11 +28,31 @@ class Loader(BaseLoader): return origin.loader.get_contents(origin) def get_template(self, template_name, template_dirs=None, skip=None): + """ + Perform the caching that gives this loader its name. Often many of the + templates attempted will be missing, so memory use is of concern here. + To keep it in check, caching behavior is a little complicated when a + template is not found. See ticket #26306 for more details. + + With template debugging disabled, cache the TemplateDoesNotExist class + for every missing template and raise a new instance of it after + fetching it from the cache. + + With template debugging enabled, a unique TemplateDoesNotExist object + is cached for each missing template to preserve debug data. When + raising an exception, Python sets __traceback__, __context__, and + __cause__ attributes on it. Those attributes can contain references to + all sorts of objects up the call chain and caching them creates a + memory leak. Thus, unraised copies of the exceptions are cached and + copies of those copies are raised after they're fetched from the cache. + """ key = self.cache_key(template_name, template_dirs, skip) cached = self.get_template_cache.get(key) if cached: - if isinstance(cached, TemplateDoesNotExist): - raise cached + if isinstance(cached, type) and issubclass(cached, TemplateDoesNotExist): + raise cached(template_name) + elif isinstance(cached, TemplateDoesNotExist): + raise copy_exception(cached) return cached try: @@ -39,7 +60,7 @@ class Loader(BaseLoader): template_name, template_dirs, skip, ) except TemplateDoesNotExist as e: - self.get_template_cache[key] = e + self.get_template_cache[key] = copy_exception(e) if self.engine.debug else TemplateDoesNotExist raise else: self.get_template_cache[key] = template diff --git a/docs/releases/1.9.5.txt b/docs/releases/1.9.5.txt index 7e8efb6fec..d74a2d9abe 100644 --- a/docs/releases/1.9.5.txt +++ b/docs/releases/1.9.5.txt @@ -25,3 +25,5 @@ Bugfixes their password to something with such whitespace after a site updated to Django 1.9 to reset their password. It provides backwards-compatibility for earlier versions of Django. + +* Fixed a memory leak in the cached template loader (:ticket:`26306`). diff --git a/tests/template_tests/test_loaders.py b/tests/template_tests/test_loaders.py index 11f20c6deb..35921e0472 100644 --- a/tests/template_tests/test_loaders.py +++ b/tests/template_tests/test_loaders.py @@ -49,11 +49,46 @@ class CachedLoaderTests(SimpleTestCase): self.assertEqual(template.origin.template_name, 'index.html') self.assertEqual(template.origin.loader, self.engine.template_loaders[0].loaders[0]) - def test_get_template_missing(self): + def test_get_template_missing_debug_off(self): + """ + With template debugging disabled, the raw TemplateDoesNotExist class + should be cached when a template is missing. See ticket #26306 and + docstrings in the cached loader for details. + """ + self.engine.debug = False with self.assertRaises(TemplateDoesNotExist): - self.engine.get_template('doesnotexist.html') - e = self.engine.template_loaders[0].get_template_cache['doesnotexist.html'] - self.assertEqual(e.args[0], 'doesnotexist.html') + self.engine.get_template('prod-template-missing.html') + e = self.engine.template_loaders[0].get_template_cache['prod-template-missing.html'] + self.assertEqual(e, TemplateDoesNotExist) + + def test_get_template_missing_debug_on(self): + """ + With template debugging enabled, a TemplateDoesNotExist instance + should be cached when a template is missing. + """ + self.engine.debug = True + with self.assertRaises(TemplateDoesNotExist): + self.engine.get_template('debug-template-missing.html') + e = self.engine.template_loaders[0].get_template_cache['debug-template-missing.html'] + self.assertIsInstance(e, TemplateDoesNotExist) + self.assertEqual(e.args[0], 'debug-template-missing.html') + + @unittest.skipIf(six.PY2, "Python 2 doesn't set extra exception attributes") + def test_cached_exception_no_traceback(self): + """ + When a TemplateDoesNotExist instance is cached, the cached instance + should not contain the __traceback__, __context__, or __cause__ + attributes that Python sets when raising exceptions. + """ + self.engine.debug = True + with self.assertRaises(TemplateDoesNotExist): + self.engine.get_template('no-traceback-in-cache.html') + e = self.engine.template_loaders[0].get_template_cache['no-traceback-in-cache.html'] + + error_msg = "Cached TemplateDoesNotExist must not have been thrown." + self.assertIsNone(e.__traceback__, error_msg) + self.assertIsNone(e.__context__, error_msg) + self.assertIsNone(e.__cause__, error_msg) @ignore_warnings(category=RemovedInDjango20Warning) def test_load_template(self):