Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed #16108 -- Fixed another race condition in the FileSystemStorage…

… backend with regard to deleting a file. Refs #16082, too. Thanks, Aymeric Augustin.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@16287 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit d34bb3c833751e49d3647ae1eea837178707d803 1 parent afca4e9
Jannis Leidel jezdez authored
9 django/core/files/storage.py
View
@@ -219,8 +219,15 @@ def _save(self, name, content):
def delete(self, name):
name = self.path(name)
# If the file exists, delete it from the filesystem.
+ # Note that there is a race between os.path.exists and os.remove:
+ # if os.remove fails with ENOENT, the file was removed
+ # concurrently, and we can continue normally.
if os.path.exists(name):
- os.remove(name)
+ try:
+ os.remove(name)
+ except OSError, e:
+ if e.errno != errno.ENOENT:
+ raise
def exists(self, name):
return os.path.exists(self.path(name))
47 tests/regressiontests/file_storage/tests.py
View
@@ -304,20 +304,21 @@ def test_makedirs_race_handling(self):
"""
File storage should be robust against directory creation race conditions.
"""
+ real_makedirs = os.makedirs
+
# Monkey-patch os.makedirs, to simulate a normal call, a raced call,
# and an error.
def fake_makedirs(path):
if path == os.path.join(self.temp_dir, 'normal'):
- os.mkdir(path)
+ real_makedirs(path)
elif path == os.path.join(self.temp_dir, 'raced'):
- os.mkdir(path)
+ real_makedirs(path)
raise OSError(errno.EEXIST, 'simulated EEXIST')
elif path == os.path.join(self.temp_dir, 'error'):
raise OSError(errno.EACCES, 'simulated EACCES')
else:
self.fail('unexpected argument %r' % path)
- real_makedirs = os.makedirs
try:
os.makedirs = fake_makedirs
@@ -333,11 +334,47 @@ def fake_makedirs(path):
# Check that OSErrors aside from EEXIST are still raised.
self.assertRaises(OSError,
- lambda: self.storage.save('error/test.file',
- ContentFile('not saved')))
+ self.storage.save, 'error/test.file', ContentFile('not saved'))
finally:
os.makedirs = real_makedirs
+ def test_remove_race_handling(self):
+ """
+ File storage should be robust against file removal race conditions.
+ """
+ real_remove = os.remove
+
+ # Monkey-patch os.remove, to simulate a normal call, a raced call,
+ # and an error.
+ def fake_remove(path):
+ if path == os.path.join(self.temp_dir, 'normal.file'):
+ real_remove(path)
+ elif path == os.path.join(self.temp_dir, 'raced.file'):
+ real_remove(path)
+ raise OSError(errno.ENOENT, 'simulated ENOENT')
+ elif path == os.path.join(self.temp_dir, 'error.file'):
+ raise OSError(errno.EACCES, 'simulated EACCES')
+ else:
+ self.fail('unexpected argument %r' % path)
+
+ try:
+ os.remove = fake_remove
+
+ self.storage.save('normal.file', ContentFile('delete normally'))
+ self.storage.delete('normal.file')
+ self.assertFalse(self.storage.exists('normal.file'))
+
+ self.storage.save('raced.file', ContentFile('delete with race'))
+ self.storage.delete('raced.file')
+ self.assertFalse(self.storage.exists('normal.file'))
+
+ # Check that OSErrors aside from ENOENT are still raised.
+ self.storage.save('error.file', ContentFile('delete with error'))
+ self.assertRaises(OSError, self.storage.delete, 'error.file')
+ finally:
+ os.remove = real_remove
+
+
class CustomStorage(FileSystemStorage):
def get_available_name(self, name):
"""
Please sign in to comment.
Something went wrong with that request. Please try again.