#26332 closed Bug (fixed)
BaseCache.get_or_set() doesn't always return a value
Reported by: | Przemysław Suliga | Owned by: | nobody |
---|---|---|---|
Component: | Core (Cache system) | Version: | 1.9 |
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 (last modified by )
BaseCache.get_or_set() returns False
if the key was set in cache between the first .get()
and .add()
.
The reason for using .add()
instead of .set()
in there is a "big race condition" as stated in #12982. However, it is very unlikely that the caller will pass a default
callable that reads the key from the cache and returns a result that depends on the value currently stored in cache which would be a prerequisite for a race condition to occur. Using set()
as originally stated in the description of #12982 means that there will be a data race which doesn't affect the correctness of the program, because the result is that multiple writes happened, but all of them are correct.
Following texts contain a nice explanation of what is a race condition and how it relates to a data race:
As a user of get_or_set()
I am expecting it to always return a value, regardless if a write to the cache happened and it is inefficient for me to handle the False
return scenario by calling my default()
callback again to obtain the value I need (it was already executed inside .get_or_set, but the result was discarded).
I can see 2 possible fixes:
- always use
.set()
and always return the value of default - ignore the result of
.add()
and always return to the user with the result of.get(key, default)
after the call to.add()
Change History (6)
comment:1 by , 9 years ago
comment:2 by , 9 years ago
Description: | modified (diff) |
---|
comment:3 by , 9 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Uncategorized → Bug |
This issue was also raised on django-developers.
comment:4 by , 9 years ago
Summary: | `BaseCache.get_or_set()` violates principle of least astonishment → BaseCache.get_or_set() doesn't always return a value |
---|
https://github.com/django/django/pull/6250 takes the second proposed approach to fix this.