#21200 closed Bug (fixed)
Inconsistent handling of "PickleError" between cache backends
Reported by: | Owned by: | ||
---|---|---|---|
Component: | Core (Cache system) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
It looks like a PickleError
raised while caching some object will get caught when using LocMemCache
(see https://github.com/django/django/blob/728548e483a5a3486939b0c8e62520296587482e/django/core/cache/backends/locmem.py#L30) whilst it won't when using DatabaseCache
(for instance, see https://github.com/django/django/blob/728548e483a5a3486939b0c8e62520296587482e/django/core/cache/backends/db.py#L114).
I can see no valid reason why the behavior regarding pickle errors should differ among different cache backends and this can lead to errors when tests do not use the same backend as production (it just happened to me with a pickle error : a class that defines __slots__ without defining __getstate__ cannot be pickled
)
Attachments (3)
Change History (15)
comment:1 Changed 10 years ago by
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Uncategorized → Bug |
comment:2 Changed 10 years ago by
comment:3 Changed 10 years ago by
Just to clarify the issue: several cache backends rely on pickle.dumps()
to serialize some object in order to save them (in database, live memory, etc.). This method can raise an exception when the object cannot be pickled, which can happen in some circumstances.
The problem is that with the LockMemCache
, when such an error occurs, the cache fails silently whereas it raises a PickleError
with some other caches (e.g. DatabaseCache
). This behavior was introduced a long time ago by ae7f04caab1b4f2a2b509b036499e4e042caaac6.
This is dangerous because someone using LockMemCache
could have systematic cache failures going silent.
In my opinion, the correct behavior is the one of DatabaseCache
since those failures should be detectable and handled by users. Maybe writing specific serialize
/deserialize
methods whith specific exceptions could help enforcing the same behavior across backends ?
Changed 10 years ago by
Attachment: | locmem_django_issue_21200.patch added |
---|
comment:4 Changed 10 years ago by
The patch above makes LocMemCache
stop failing silently in case of pickling error, which is consistent with other backends.
comment:5 Changed 10 years ago by
Has patch: | set |
---|
comment:6 Changed 10 years ago by
Needs documentation: | set |
---|---|
Needs tests: | set |
Could you add a regression test that demonstrates PickleError
is raised when using LocMemCache
. It probably also warrants a mention in the release notes as a backwards incompatible change.
Changed 10 years ago by
Attachment: | locmem_django_issue_21200_with_tests.patch added |
---|
Same as above, with tests
comment:7 Changed 10 years ago by
Needs tests: | unset |
---|
I added some tests covering every backend and failing specifically for the LocMemCache without the included modification.
Changed 10 years ago by
Attachment: | release_note_issue21200.patch added |
---|
Release note patch (backwards incompatible change)
comment:8 Changed 10 years ago by
Needs documentation: | unset |
---|
comment:9 Changed 10 years ago by
Owner: | changed from nobody to Aymeric Augustin |
---|---|
Status: | new → assigned |
comment:10 Changed 10 years ago by
Owner: | Aymeric Augustin deleted |
---|---|
Status: | assigned → new |
Handing this ticket over to apollo13 :P
comment:11 Changed 10 years ago by
Owner: | set to Florian Apolloner <florian@…> |
---|---|
Resolution: | → fixed |
Status: | new → closed |
comment:12 Changed 8 years ago by
The above commit only included testes and docs -- cf7ddc576583a15dec0800f4146c2e979795bf30 included the code changes.
Looking at the history of locmem.py, I found commit 76ee39ce14b851a8247c3111de0fbc91db4de1b1 which seems relevant to this issue (it's a fix for #20613).
If I understand correctly, it actually changed the handling of a PickleError (from
return True
toreturn False
) and I'm not sure if this was intended or not.