[Django] #34209: FileBasedCache has_key is susceptible to race conditions

[Django]

#34209: FileBasedCache has_key is susceptible to race conditions
-------------------------------------+-------------------------------------
Reporter: Marti | Owner: nobody
Raudsepp |
Type: | Status: new
Uncategorized |
Component: Core | Version: 4.1
(Cache system) | Keywords: race, race-
Severity: Normal | condition, FileBasedCache
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I received the exception from Django's cache framework:

{{{
FileNotFoundError: [Errno 2] No such file or directory:
'/app/var/cache/d729e4cf4ba88cba5a0f48e0396ec48a.djcache'
[...]
File "django/core/cache/backends/base.py", line 229, in get_or_set
self.add(key, default, timeout=timeout, version=version)
File "django/core/cache/backends/filebased.py", line 26, in add
if self.has_key(key, version):
File "django/core/cache/backends/filebased.py", line 94, in has_key
with open(fname, "rb") as f:
}}}

The code is:
{{{
def has_key(self, key, version=None):
fname = self._key_to_file(key, version)
if os.path.exists(fname):
with open(fname, "rb") as f:
return not self._is_expired(f)
return False
}}}

Between the `exists()` check and `open()`, it's possible for the file to
be deleted. In fact, the `_is_expired()` method itself deletes the file if
it finds it to be expired. So if many threads race to read an expired
cache at once, it's not that unlikely to hit this window.

--
Ticket URL: <https://code.djangoproject.com/ticket/34209>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#34209: FileBasedCache has_key is susceptible to race conditions
-------------------------------------+-------------------------------------

Reporter: Marti Raudsepp | Owner: Marti
| Raudsepp
Type: Uncategorized | Status: assigned
Component: Core (Cache system) | Version: 4.1
Severity: Normal | Resolution:
Keywords: race, race- | Triage Stage:
condition, FileBasedCache | Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Marti Raudsepp):

* owner: nobody => Marti Raudsepp
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/34209#comment:1>

[Django]

#34209: FileBasedCache has_key is susceptible to race conditions
-------------------------------------+-------------------------------------
Reporter: Marti Raudsepp | Owner: Marti
| Raudsepp
Type: Uncategorized | Status: assigned
Component: Core (Cache system) | Version: 4.1
Severity: Normal | Resolution:
Keywords: race, race- | Triage Stage:
condition, FileBasedCache | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Description changed by Marti Raudsepp:

Old description:

> I received the exception from Django's cache framework:
>
> {{{
> FileNotFoundError: [Errno 2] No such file or directory:
> '/app/var/cache/d729e4cf4ba88cba5a0f48e0396ec48a.djcache'
> [...]
> File "django/core/cache/backends/base.py", line 229, in get_or_set
> self.add(key, default, timeout=timeout, version=version)
> File "django/core/cache/backends/filebased.py", line 26, in add
> if self.has_key(key, version):
> File "django/core/cache/backends/filebased.py", line 94, in has_key
> with open(fname, "rb") as f:
> }}}
>
> The code is:
> {{{
> def has_key(self, key, version=None):
> fname = self._key_to_file(key, version)
> if os.path.exists(fname):
> with open(fname, "rb") as f:
> return not self._is_expired(f)
> return False
> }}}
>
> Between the `exists()` check and `open()`, it's possible for the file to
> be deleted. In fact, the `_is_expired()` method itself deletes the file
> if it finds it to be expired. So if many threads race to read an expired
> cache at once, it's not that unlikely to hit this window.

New description:

I received the exception from Django's cache framework:

{{{
FileNotFoundError: [Errno 2] No such file or directory:
'/app/var/cache/d729e4cf4ba88cba5a0f48e0396ec48a.djcache'
[...]
File "django/core/cache/backends/base.py", line 229, in get_or_set
self.add(key, default, timeout=timeout, version=version)
File "django/core/cache/backends/filebased.py", line 26, in add
if self.has_key(key, version):
File "django/core/cache/backends/filebased.py", line 94, in has_key
with open(fname, "rb") as f:
}}}

The code is:
{{{#!python

def has_key(self, key, version=None):
fname = self._key_to_file(key, version)
if os.path.exists(fname):
with open(fname, "rb") as f:
return not self._is_expired(f)
return False
}}}

Between the `exists()` check and `open()`, it's possible for the file to
be deleted. In fact, the `_is_expired()` method itself deletes the file if
it finds it to be expired. So if many threads race to read an expired
cache at once, it's not that unlikely to hit this window.

--

--
Ticket URL: <https://code.djangoproject.com/ticket/34209#comment:2>

[Django]

#34209: FileBasedCache has_key is susceptible to race conditions
-------------------------------------+-------------------------------------
Reporter: Marti Raudsepp | Owner: Marti
| Raudsepp

Type: Bug | Status: assigned

Component: Core (Cache system) | Version: 4.1
Severity: Normal | Resolution:

Keywords: race, race- | Triage Stage: Accepted
condition, FileBasedCache |

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* type: Uncategorized => Bug
* stage: Unreviewed => Accepted

Comment:

Thanks for the report.

--
Ticket URL: <https://code.djangoproject.com/ticket/34209#comment:3>

[Django]

#34209: FileBasedCache has_key is susceptible to race conditions
-------------------------------------+-------------------------------------
Reporter: Marti Raudsepp | Owner: Marti
| Raudsepp
Type: Bug | Status: assigned
Component: Core (Cache system) | Version: 4.1
Severity: Normal | Resolution:
Keywords: race, race- | Triage Stage: Accepted
condition, FileBasedCache |
Has patch: 1 | Needs documentation: 0

Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* needs_tests: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/34209#comment:4>

[Django]

#34209: FileBasedCache has_key is susceptible to race conditions
-------------------------------------+-------------------------------------
Reporter: Marti Raudsepp | Owner: Marti
| Raudsepp
Type: Bug | Status: assigned
Component: Core (Cache system) | Version: 4.1
Severity: Normal | Resolution:
Keywords: race, race- | Triage Stage: Accepted
condition, FileBasedCache |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Marti Raudsepp):

Added a test.

--
Ticket URL: <https://code.djangoproject.com/ticket/34209#comment:5>

[Django]

#34209: FileBasedCache has_key is susceptible to race conditions
-------------------------------------+-------------------------------------
Reporter: Marti Raudsepp | Owner: Marti
| Raudsepp
Type: Bug | Status: assigned
Component: Core (Cache system) | Version: 4.1
Severity: Normal | Resolution:

Keywords: race, race- | Triage Stage: Ready for
condition, FileBasedCache | checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* needs_tests: 1 => 0
* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/34209#comment:6>

[Django]

#34209: FileBasedCache has_key is susceptible to race conditions
-------------------------------------+-------------------------------------
Reporter: Marti Raudsepp | Owner: Marti
| Raudsepp

Type: Bug | Status: closed

Component: Core (Cache system) | Version: 4.1

Severity: Normal | Resolution: fixed

Keywords: race, race- | Triage Stage: Ready for
condition, FileBasedCache | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"32268456d6494483f9a737dc7c32bbbf1eceab8c" 32268456]:
{{{
#!CommitTicketReference repository=""
revision="32268456d6494483f9a737dc7c32bbbf1eceab8c"
Fixed #34209 -- Prevented FileBasedCache.has_key() crash caused by a race
condition.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/34209#comment:7>

[Django]

#34209: FileBasedCache has_key is susceptible to race conditions
-------------------------------------+-------------------------------------
Reporter: Marti Raudsepp | Owner: Marti
| Raudsepp
Type: Bug | Status: closed
Component: Core (Cache system) | Version: 4.1
Severity: Normal | Resolution: fixed
Keywords: race, race- | Triage Stage: Ready for
condition, FileBasedCache | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Marti Raudsepp):

This fix doesn't qualify for backporting?

--
Ticket URL: <https://code.djangoproject.com/ticket/34209#comment:8>