#36662: django.contrib.messages Storage creates circular references with Request
objects
-------------------------------------+-------------------------------------
Reporter: Raphael Gaschignard | Owner: Augusto
Type: | Pontes
Cleanup/optimization | Status: assigned
Component: contrib.messages | Version: dev
Severity: Normal | Resolution:
Keywords: gc, garbage, | Triage Stage: Accepted
weakref |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):
* keywords: => gc, garbage, weakref
* stage: Unreviewed => Accepted
Comment:
Thanks, I reproduced by riffing on the test from #34865:
{{{#!diff
diff --git a/tests/messages_tests/base.py b/tests/messages_tests/base.py
index ce4b2acac8..b512788ec0 100644
--- a/tests/messages_tests/base.py
+++ b/tests/messages_tests/base.py
@@ -330,6 +330,22 @@ class BaseTests:
self.assertEqual(len(storage), 6)
def test_high_level(self):
+ import gc
+
+ # Schedule the restore of the garbage collection settings.
+ self.addCleanup(gc.set_debug, 0)
+ self.addCleanup(gc.enable)
+
+ # Disable automatic garbage collection to control when it's
triggered,
+ # then run a full collection cycle to ensure `gc.garbage` is
empty.
+ gc.disable()
+ gc.collect()
+
+ # The garbage list isn't automatically populated to avoid CPU
overhead,
+ # so debugging needs to be enabled to track all unreachable items
and
+ # have them stored in `gc.garbage`.
+ gc.set_debug(gc.DEBUG_SAVEALL)
+
request = self.get_request()
storage = self.storage_class(request)
request._messages = storage
@@ -340,6 +356,15 @@ class BaseTests:
add_level_messages(storage)
self.assertEqual(len(storage), 2)
+ # Dereference.
+ request = None
+ storage = None
+
+ # Enforce garbage collection to populate `gc.garbage` for
inspection.
+ gc.collect()
+ self.assertEqual(gc.garbage, [])
+
+
@override_settings(MESSAGE_LEVEL=29)
def test_settings_level(self):
request = self.get_request()
}}}
{{{#!py
AssertionError: Lists differ: [<HttpRequest>, <QueryDict: {}>,
<QueryDic[237 chars], []] != []
First list contains 11 additional elements.
First extra element 0:
<HttpRequest>
+ []
- [<HttpRequest>,
- <QueryDict: {}>,
- <QueryDict: {}>,
- {},
- {},
- <MultiValueDict: {}>,
- <SessionStorage: request=<HttpRequest>>,
- [Message(level=30, message='A warning'),
- Message(level=40, message='An error')],
- Message(level=30, message='A warning'),
- Message(level=40, message='An error'),
- []]
}}}
> The "simple" fix here would be to hold onto request in the storage
through a weakref, and add a property to get the request object on the
storage class.
I tried that:
{{{#!diff
diff --git a/django/contrib/messages/storage/base.py
b/django/contrib/messages/storage/base.py
index 5d89acfe69..9bca9e04c2 100644
--- a/django/contrib/messages/storage/base.py
+++ b/django/contrib/messages/storage/base.py
@@ -1,3 +1,4 @@
+import weakref
from django.conf import settings
from django.contrib.messages import constants, utils
from django.utils.functional import SimpleLazyObject
@@ -55,12 +56,16 @@ class BaseStorage:
"""
def __init__(self, request, *args, **kwargs):
- self.request = request
+ self._request = weakref.proxy(request)
self._queued_messages = []
self.used = False
self.added_new = False
super().__init__(*args, **kwargs)
+ @property
+ def request(self):
+ return self._request
+
def __len__(self):
return len(self._loaded_messages) + len(self._queued_messages)
}}}
But I just got:
{{{#!py
ERROR: test_add_lazy_translation
(messages_tests.test_session.SessionTests.test_add_lazy_translation)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/jwalls/django/tests/messages_tests/base.py", line 101, in
test_add_lazy_translation
storage.update(response)
~~~~~~~~~~~~~~^^^^^^^^^^
File "/Users/jwalls/django/django/contrib/messages/storage/base.py",
line 145, in update
return self._store(messages, response)
~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^
File "/Users/jwalls/django/django/contrib/messages/storage/session.py",
line 40, in _store
self.request.session[self.session_key] =
self.serialize_messages(messages)
^^^^^^^^^^^^^^^^^^^^
ReferenceError: weakly-referenced object no longer exists
----------------------------------------------------------------------
Ran 4 tests in 0.008s
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36662#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion visit
https://groups.google.com/d/msgid/django-updates/0107019a03dea0e2-1cc246ad-34b9-4e97-bb3d-3a47a0a36a01-000000%40eu-central-1.amazonses.com.
