#16082 closed Bug (fixed)
Directory creation race condition in FileSystemStorage
Reported by: | Pi Delport | Owned by: | nobody |
---|---|---|---|
Component: | File uploads/storage | Version: | dev |
Severity: | Normal | Keywords: | FileSystemStorage directory creation race condition |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
My company recently encountered the following problem on a production server:
Traceback (most recent call last): [...] File "[...]", line 191, in process_view storage.save(path, File([...])) File "[...]/python2.6/site-packages/django/core/files/storage.py", line 47, in save name = self._save(name, content) File "[...]/python2.6/site-packages/django/core/files/storage.py", line 146, in _save os.makedirs(directory) File "[...]/python2.6/os.py", line 157, in makedirs mkdir(name, mode) OSError: [Errno 17] File exists: '[MEDIA_ROOT]/avatars/foo'
I tracked this down to a race condition in FileSystemStorage._save
. When there is enough load, two concurrent processes can attempt to create the same intermediate directory, causing the second one to fail entirely.
To handle this robustly, FileSystemStorage
should catch the EEXIST
from os.makedirs
and continue normally: I implemented this along with some tests, in the attached patch.
Note: For comparison, Python 3.2 added a keyword argument to os.makedirs
to do this: see Issue 9299: os.makedirs(): Add a keyword argument to suppress "File exists" exception.
Attachments (1)
Change History (7)
by , 13 years ago
Attachment: | django-file_storage-makedirs-race.patch added |
---|
comment:1 by , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 13 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
The patch looks good and contains extensive tests. It applies cleanly and the test suite passes. The implementation doesn't add much overhead. It's a fix for an edge case, so it does not deserve a mention in the release notes.