diff options
| -rw-r--r-- | django/core/files/storage.py | 9 | ||||
| -rw-r--r-- | tests/regressiontests/file_storage/tests.py | 47 |
2 files changed, 50 insertions, 6 deletions
diff --git a/django/core/files/storage.py b/django/core/files/storage.py index f29d16c985..7590171259 100644 --- a/django/core/files/storage.py +++ b/django/core/files/storage.py @@ -219,8 +219,15 @@ class FileSystemStorage(Storage): 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)) diff --git a/tests/regressiontests/file_storage/tests.py b/tests/regressiontests/file_storage/tests.py index f498f1f0e0..917d2f36fc 100644 --- a/tests/regressiontests/file_storage/tests.py +++ b/tests/regressiontests/file_storage/tests.py @@ -304,20 +304,21 @@ class FileStorageTests(unittest.TestCase): """ 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 @@ class FileStorageTests(unittest.TestCase): # 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): """ |