Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#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 Przemysław Suliga)

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 Changed 5 years ago by Przemysław Suliga

https://github.com/django/django/pull/6250 takes the second proposed approach to fix this.

comment:2 Changed 5 years ago by Przemysław Suliga

Description: modified (diff)

comment:3 Changed 5 years ago by Tim Graham

Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

This issue was also raised on django-developers.

comment:4 Changed 5 years ago by Tim Graham

Summary: `BaseCache.get_or_set()` violates principle of least astonishmentBaseCache.get_or_set() doesn't always return a value

comment:5 Changed 5 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 96ec67a:

Fixed #26332 -- Fixed a race condition in BaseCache.get_or_set().

comment:6 Changed 5 years ago by Tim Graham <timograham@…>

In 76926f3:

[1.9.x] Fixed #26332 -- Fixed a race condition in BaseCache.get_or_set().

Backport of 96ec67a7cf89a136e793305343c5bba8521cdb47 from master

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