Opened 10 years ago
Last modified 10 years ago
#26332 closed Bug
`BaseCache.get_or_set()` violates principle of least astonishment — at Version 2
| 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 (2)
comment:1 by , 10 years ago
comment:2 by , 10 years ago
| Description: | modified (diff) |
|---|
https://github.com/django/django/pull/6250 takes the second proposed approach to fix this.