Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#25840 closed Bug (fixed)

cache.get_or_set() works incorrectly with DummyCache backend

Reported by: Oleksiy Ivanenko Owned by: nobody
Component: Core (Cache system) Version: 1.9
Severity: Normal Keywords: get_or_set dummycache
Cc: oleksiy.ivanenko@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

There is some issue when you use django.core.cache.cache.get_or_set with DummyCache backend.

from django.core.cache import cache

cache.get_or_set('some_key', 'my_val')  # Always returns None. Not a default value

This thing brakes tests.
I think it will be better to return default value from dummy cache.

Attachments (1)

get_or_set_dummycache.path (782 bytes) - added by Oleksiy Ivanenko 4 years ago.

Download all attachments as: .zip

Change History (8)

Changed 4 years ago by Oleksiy Ivanenko

Attachment: get_or_set_dummycache.path added

comment:1 Changed 4 years ago by Tim Graham

Needs tests: set
Summary: cache.get_or_set works incorrectly with DummyCache backendcache.get_or_set() works incorrectly with DummyCache backend
Triage Stage: UnreviewedAccepted

A regression test is also required. If you can submit your patch as a pull request, that's ideal. Thanks!

comment:2 Changed 4 years ago by David Szotten

one might argue that this is a bug in the BaseCache implementation of get_or_set:

def get_or_set(...)
    # [snip]
    val = self.add(key, default, timeout=timeout, version=version)
    if val:
        # unlikely, but we might have been added, and evicted again by the time we get here,
        # should we not call `get` with the default?
        return self.get(key, version=version)

comment:3 Changed 4 years ago by Oleksiy Ivanenko

My solution provided as PR - https://github.com/django/django/pull/5751

comment:4 Changed 4 years ago by Emre Yılmaz

Missed the above PR, but here is my approach. (different solution and missing unit tests included,)

https://github.com/django/django/pull/5756

Last edited 4 years ago by Emre Yılmaz (previous) (diff)

comment:5 Changed 4 years ago by Tim Graham

Needs tests: unset

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

Resolution: fixed
Status: newclosed

In 8e838d9:

Fixed #25840 -- Fixed BaseCache.get_or_set() on the DummyCache backend.

This also fixes a possible data eviction race condition between
setting and getting a key. Another thread could remove the key
before get_and_set() accesses it again. In this case, now the
default value will be returned instead of None.

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

In 9733ff5:

[1.9.x] Fixed #25840 -- Fixed BaseCache.get_or_set() on the DummyCache backend.

This also fixes a possible data eviction race condition between
setting and getting a key. Another thread could remove the key
before get_and_set() accesses it again. In this case, now the
default value will be returned instead of None.

Backport of 8e838d9c869083597dc9e161ae2fe37acaa90de9 from master

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