Inconsistent handling of "PickleError" between cache backends

[t.chaumeny@…]

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)

[Baptiste Mispelon]

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 to return False) and I'm not sure if this was intended or not.

[Thomas C]

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 an 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 ?

[Thomas C]

The patch above makes LocMemCache stop failing silently in case of pickling error, which is consistent with other backends.

[Tim Graham]

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.

[Thomas C]

Same as above, with tests

[Thomas C]

I added some tests covering every backend and failing specifically for the LocMemCache without the included modification.

[Thomas C]

Release note patch (backwards incompatible change)

[Aymeric Augustin]

Handing this ticket over to apollo13 :P

[Florian Apolloner <florian@…>]

In e112654fc81ddb3fbffbb8382b004d69367a85fe:

Fixed #21200 -- Consistantly raise errors across all cache backends.

Thanks to tchaumeny for the patch.

[Tim Graham]

The above commit only included testes and docs -- cf7ddc576583a15dec0800f4146c2e979795bf30 included the code changes.