From: Armin Rigo Date: Thu, 13 Apr 2017 18:08:15 +0000 (+0200) Subject: bpo-29694: race condition in pathlib mkdir with flags parents=True (GH-1089) X-Git-Tag: v3.7.0a1~969 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=22a594a0047d7706537ff2ac676cdc0f1dcb329c;p=python bpo-29694: race condition in pathlib mkdir with flags parents=True (GH-1089) --- diff --git a/Lib/pathlib.py b/Lib/pathlib.py index fc7ce5eb2a..19142295c9 100644 --- a/Lib/pathlib.py +++ b/Lib/pathlib.py @@ -1217,8 +1217,8 @@ class Path(PurePath): 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 diff --git a/Lib/test/test_pathlib.py b/Lib/test/test_pathlib.py index 46a705e1cf..21a6390e35 100644 --- a/Lib/test/test_pathlib.py +++ b/Lib/test/test_pathlib.py @@ -8,6 +8,7 @@ import socket import stat import tempfile import unittest +from unittest import mock from test import support android_not_root = support.android_not_root @@ -1801,6 +1802,35 @@ class _BasePathTest(object): 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) diff --git a/Misc/NEWS b/Misc/NEWS index ec85455007..9b5150c834 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -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.