Skip to content

bpo-29694: race condition in pathlib mkdir with flags parents=True #1089

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Lib/pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -1217,8 +1217,8 @@ def mkdir(self, mode=0o777, parents=False, exist_ok=False):
except FileNotFoundError:
if not parents or self.parent == self:
raise
self.parent.mkdir(parents=True)
self._accessor.mkdir(self, mode)
self.parent.mkdir(parents=True, exist_ok=True)
self.mkdir(mode, parents=False, exist_ok=exist_ok)
except OSError:
# Cannot rely on checking for EEXIST, since the operating system
# could give priority to other errors like EACCES or EROFS
Expand Down
30 changes: 30 additions & 0 deletions Lib/test/test_pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import stat
import tempfile
import unittest
from unittest import mock

from test import support
android_not_root = support.android_not_root
Expand Down Expand Up @@ -1801,6 +1802,35 @@ def test_mkdir_no_parents_file(self):
p.mkdir(exist_ok=True)
self.assertEqual(cm.exception.errno, errno.EEXIST)

def test_mkdir_concurrent_parent_creation(self):
for pattern_num in range(32):
p = self.cls(BASE, 'dirCPC%d' % pattern_num)
self.assertFalse(p.exists())

def my_mkdir(path, mode=0o777):
path = str(path)
# Emulate another process that would create the directory
# just before we try to create it ourselves. We do it
# in all possible pattern combinations, assuming that this
# function is called at most 5 times (dirCPC/dir1/dir2,
# dirCPC/dir1, dirCPC, dirCPC/dir1, dirCPC/dir1/dir2).
if pattern.pop():
os.mkdir(path, mode) # from another process
concurrently_created.add(path)
os.mkdir(path, mode) # our real call

pattern = [bool(pattern_num & (1 << n)) for n in range(5)]
concurrently_created = set()
p12 = p / 'dir1' / 'dir2'
try:
with mock.patch("pathlib._normal_accessor.mkdir", my_mkdir):
p12.mkdir(parents=True, exist_ok=False)
except FileExistsError:
self.assertIn(str(p12), concurrently_created)
else:
self.assertNotIn(str(p12), concurrently_created)
self.assertTrue(p.exists())

@support.skip_unless_symlink
def test_symlink_to(self):
P = self.cls(BASE)
Expand Down
4 changes: 4 additions & 0 deletions Misc/NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,10 @@ Extension Modules

Library
-------

- bpo-29694: Fixed race condition in pathlib mkdir with flags
parents=True. Patch by Armin Rigo.

- bpo-29692: Fixed arbitrary unchaining of RuntimeError exceptions in
contextlib.contextmanager.
Patch by Siddharth Velankar.
Expand Down