Opened 6 years ago

Last modified 6 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()

Change History (0)

Note: See TracTickets for help on using tickets.
Back to Top