Merge lp:~matt-goodall/python-oops-datedir-repo/concurrent-makedirs-race into lp:python-oops-datedir-repo

Proposed by Matt Goodall
Status: Merged
Merged at revision: 49
Proposed branch: lp:~matt-goodall/python-oops-datedir-repo/concurrent-makedirs-race
Merge into: lp:python-oops-datedir-repo
Diff against target: 44 lines (+18/-1)
2 files modified
oops_datedir_repo/repository.py (+6/-1)
oops_datedir_repo/tests/test_repository.py (+12/-0)
To merge this branch: bzr merge lp:~matt-goodall/python-oops-datedir-repo/concurrent-makedirs-race
Reviewer Review Type Date Requested Status
Vincent Ladeuil (community) Approve
Launchpad code reviewers Pending
Review via email: mp+275689@code.launchpad.net

Commit message

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

Description of the change

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

To post a comment you must log in.
Revision history for this message
Vincent Ladeuil (vila) wrote :

Nice catch.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'oops_datedir_repo/repository.py'
2--- oops_datedir_repo/repository.py 2013-03-12 14:37:56 +0000
3+++ oops_datedir_repo/repository.py 2015-10-26 15:02:50 +0000
4@@ -108,7 +108,12 @@
5 oopsid = 'OOPS-%s' % md5hash
6 prefix = os.path.join(self.root, now.strftime('%Y-%m-%d'))
7 if not os.path.isdir(prefix):
8- os.makedirs(prefix)
9+ try:
10+ os.makedirs(prefix)
11+ except OSError as err:
12+ # EEXIST - dir created by another, concurrent process
13+ if err.errno != errno.EEXIST:
14+ raise
15 # For directories we need to set the x bits too.
16 os.chmod(
17 prefix, wanted_file_permission | stat.S_IXUSR | stat.S_IXGRP |
18
19=== modified file 'oops_datedir_repo/tests/test_repository.py'
20--- oops_datedir_repo/tests/test_repository.py 2013-03-12 14:37:56 +0000
21+++ oops_datedir_repo/tests/test_repository.py 2015-10-26 15:02:50 +0000
22@@ -24,6 +24,7 @@
23
24 from fixtures import (
25 Fixture,
26+ MonkeyPatch,
27 TempDir,
28 )
29 from pytz import utc
30@@ -394,3 +395,14 @@
31 repo.publish(missingtime, now)
32 repo.prune_unreferenced(old, now, [])
33 self.assertThat(lambda: repo.oldest_date(), raises(ValueError))
34+
35+ def test_concurrent_dir_creation(self):
36+ # Simulate isdir/makedirs race condition where concurrent processes
37+ # test and see no existing dir, all try to create it, and first process
38+ # wins causing others to fail.
39+ def fake_isdir(path):
40+ return False
41+ self.useFixture(MonkeyPatch('os.path.isdir', fake_isdir))
42+ repo = DateDirRepo(self.useFixture(TempDir()).path)
43+ repo.publish({'id': '1'})
44+ repo.publish({'id': '2'})

Subscribers

People subscribed via source and target branches

to all changes: