[Django] #25840: cache.get_or_set works incorrectly with DummyCache backend
Django
-------------------------------------+-----------------------------------
Reporter: oleksiyivanenko | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: 1.9
Severity: Normal | Keywords: get_or_set dummycache
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-----------------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25840>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: oleksiyivanenko | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: 1.9
Keywords: get_or_set dummycache | Triage Stage: Unreviewed
Has patch: 1 | Easy pickings: 0
UI/UX: 0 |
Changes (by oleksiyivanenko):
* Attachment "get_or_set_dummycache.path" added.
Django
-------------------------------------+-------------------------------------
Reporter: oleksiyivanenko | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: 1.9
Keywords: get_or_set | Triage Stage: Accepted
dummycache |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* needs_tests: => 1
* needs_docs: => 0
Comment:
A regression test is also required. If you can submit your patch as a pull
request, that's ideal. Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/25840#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: oleksiyivanenko | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: 1.9
Keywords: get_or_set | Triage Stage: Accepted
dummycache |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Comment (by davidszotten):
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)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25840#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: oleksiyivanenko | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: 1.9
Keywords: get_or_set | Triage Stage: Accepted
dummycache |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Comment (by oleksiyivanenko):
My solution provided as PR - https://github.com/django/django/pull/5751
--
Ticket URL: <https://code.djangoproject.com/ticket/25840#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: oleksiyivanenko | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: 1.9
Keywords: get_or_set | Triage Stage: Accepted
dummycache |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Comment (by emre):
Missed the above PR, but here is my approach. (different solution)
https://github.com/django/django/pull/5756
--
Ticket URL: <https://code.djangoproject.com/ticket/25840#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: oleksiyivanenko | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: 1.9
Keywords: get_or_set | Triage Stage: Accepted
dummycache |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/25840#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: oleksiyivanenko | Owner: nobody
Type: Bug | Status: closed
Component: Core (Cache system) | Version: 1.9
Keywords: get_or_set | Triage Stage: Accepted
dummycache |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"8e838d9c869083597dc9e161ae2fe37acaa90de9" 8e838d9]:
{{{
#!CommitTicketReference repository=""
revision="8e838d9c869083597dc9e161ae2fe37acaa90de9"
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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25840#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: oleksiyivanenko | Owner: nobody
Type: Bug | Status: closed
Component: Core (Cache system) | Version: 1.9
Keywords: get_or_set | Triage Stage: Accepted
dummycache |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"9733ff5f991356ac98d499f24f2241141863a555" 9733ff5]:
{{{
#!CommitTicketReference repository=""
revision="9733ff5f991356ac98d499f24f2241141863a555"
[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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25840#comment:7>