Be resilient against parallel directory creations. (#75)

* Be resilient against parallel directory creations.

When using multiprocessing, simultaneous writes can lead to the
situation where both code paths first check if the directory exists and
create it in the negative case. As check and creation aren't a single
atomic operation, one of them might fail.

* Add test case to check concurrent failure scenario

* Assert correct Error in Python 2

* Amend changelog

docs/changes.rst

@@ -4,6 +4,7 @@ Changelog
 0.11.9
 ======
 * Add option to set the checksum for Azure blobs.
+* Make the FilesystemStore resilient to parallel directory creations.
 
 0.11.8
 ======

simplekv/fs.py

@@ -99,7 +99,11 @@ def _copy(self, source, dest):
 
     def _ensure_dir_exists(self, path):
         if not os.path.isdir(path):
-            os.makedirs(path)
+            try:
+                os.makedirs(path)
+            except OSError as e:
+                if not os.path.isdir(path):
+                    raise e
 
     def _put_file(self, key, file):
         bufsize = self.bufsize

tests/test_filesystem_store.py

@@ -2,7 +2,7 @@
 
 import os
 import stat
-from simplekv._compat import BytesIO, url_quote, url_unquote
+from simplekv._compat import BytesIO, url_quote, url_unquote, PY2
 import tempfile
 from simplekv._compat import urlparse
 
@@ -31,6 +31,28 @@ def store(self, tmpdir):
         return FilesystemStore(tmpdir)
 
 
+class TestFilesystemStoreMkdir(TestBaseFilesystemStore):
+
+    def test_concurrent_mkdir(self, tmpdir, mocker):
+        # Concurrent instantiation of the store in two threads could lead to
+        # the situation where both threads see that the directory does not
+        # exists. For one, the call to mkdir succeeds, for the other it fails.
+        # This is ok for us as long as the directory exists afterwards.
+        makedirs = mocker.patch('os.makedirs')
+        makedirs.side_effect = OSError("Failure")
+        mocker.patch('os.path.isdir')
+
+        store = FilesystemStore(os.path.join(tmpdir, 'test'))
+        # We have mocked os.makedirs, so this won't work. But it should
+        # pass beyond the OS error and simply fail on writing the file itself.
+        if PY2:
+            with pytest.raises(IOError):
+                store.put('test', b'test')
+        else:
+            with pytest.raises(FileNotFoundError):
+                store.put('test', b'test')
+
+
 class TestFilesystemStoreFileURI(TestBaseFilesystemStore):
     @pytest.mark.skipif(os.name != 'posix',
                         reason='Not supported outside posix.')

tox.ini

@@ -9,6 +9,7 @@ deps=
   pytest
   pytest-pep8
   pytest-cov
+  pytest-mock
   pytest-xdist
   mock
   tempdir