Opened 9 years ago
Last modified 9 years ago
#26332 closed Bug
`BaseCache.get_or_set()` violates principle of least astonishment — at Initial Version
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
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()