launchpad-reviewers team mailing list archive

[Matt Goodall <matt.goodall@xxxxxxxxxxxxx>]

Matt Goodall has proposed merging lp:~matt-goodall/python-oops-datedir-repo/concurrent-makedirs-race into lp:python-oops-datedir-repo.

Commit message:
Fix race when concurrent processes try to create the oops dir.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~matt-goodall/python-oops-datedir-repo/concurrent-makedirs-race/+merge/275689

Fix race when concurrent processes try to create the oops dir.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~matt-goodall/python-oops-datedir-repo/concurrent-makedirs-race into lp:python-oops-datedir-repo.

=== modified file 'oops_datedir_repo/repository.py'
--- oops_datedir_repo/repository.py	2013-03-12 14:37:56 +0000
+++ oops_datedir_repo/repository.py	2015-10-26 15:02:50 +0000
@@ -108,7 +108,12 @@
         oopsid = 'OOPS-%s' % md5hash
         prefix = os.path.join(self.root, now.strftime('%Y-%m-%d'))
         if not os.path.isdir(prefix):
-            os.makedirs(prefix)
+            try:
+                os.makedirs(prefix)
+            except OSError as err:
+                # EEXIST - dir created by another, concurrent process
+                if err.errno != errno.EEXIST:
+                    raise
             # For directories we need to set the x bits too.
             os.chmod(
                 prefix, wanted_file_permission | stat.S_IXUSR | stat.S_IXGRP |

=== modified file 'oops_datedir_repo/tests/test_repository.py'
--- oops_datedir_repo/tests/test_repository.py	2013-03-12 14:37:56 +0000
+++ oops_datedir_repo/tests/test_repository.py	2015-10-26 15:02:50 +0000
@@ -24,6 +24,7 @@
 
 from fixtures import (
     Fixture,
+    MonkeyPatch,
     TempDir,
     )
 from pytz import utc
@@ -394,3 +395,14 @@
         repo.publish(missingtime, now)
         repo.prune_unreferenced(old, now, [])
         self.assertThat(lambda: repo.oldest_date(), raises(ValueError))
+
+    def test_concurrent_dir_creation(self):
+        # Simulate isdir/makedirs race condition where concurrent processes
+        # test and see no existing dir, all try to create it, and first process
+        # wins causing others to fail.
+        def fake_isdir(path):
+            return False
+        self.useFixture(MonkeyPatch('os.path.isdir', fake_isdir))
+        repo = DateDirRepo(self.useFixture(TempDir()).path)
+        repo.publish({'id': '1'})
+        repo.publish({'id': '2'})