From 942a0db52c72f87d2abbb1f0a596eaafa4ac879b Mon Sep 17 00:00:00 2001 From: Itai Steinherz Date: Tue, 3 May 2022 02:19:13 +0300 Subject: [PATCH 1/2] [3.9] bpo-46785: Fix race condition between os.stat() and unlink on Windows (GH-31858). (cherry picked from commit 39e6b8ae6a5b49bb23746fdcc354d148ff2d98e3) Co-authored-by: Itai Steinherz --- Lib/test/test_os.py | 48 +++++++++++++++++++ Misc/ACKS | 1 + .../2022-03-13-20-35-41.bpo-46785.Pnknyl.rst | 1 + Modules/posixmodule.c | 12 ++++- 4 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Windows/2022-03-13-20-35-41.bpo-46785.Pnknyl.rst diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index e48157a3de26d4..07499b7e5b04ab 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -23,6 +23,7 @@ import sys import sysconfig import tempfile +import textwrap import threading import time import types @@ -2714,7 +2715,54 @@ def test_getfinalpathname_handles(self): self.assertEqual(0, handle_delta) +<<<<<<< HEAD @support.skip_unless_symlink +======= + @support.requires_subprocess() + def test_stat_unlink_race(self): + # bpo-46785: the implementation of os.stat() falls back to reading + # the parent directory if CreateFileW() fails with a permission + # error. If reading the parent directory fails because the file or + # directory are subsequently unlinked, or because the volume or + # share are no longer available, then the original permission error + # should not be restored. + filename = os_helper.TESTFN + self.addCleanup(os_helper.unlink, filename) + deadline = time.time() + 5 + command = textwrap.dedent("""\ + import os + import sys + import time + + filename = sys.argv[1] + deadline = float(sys.argv[2]) + + while time.time() < deadline: + try: + with open(filename, "w") as f: + pass + except OSError: + pass + try: + os.remove(filename) + except OSError: + pass + """) + + with subprocess.Popen([sys.executable, '-c', command, filename, str(deadline)]) as proc: + while time.time() < deadline: + try: + os.stat(filename) + except FileNotFoundError as e: + assert e.winerror == 2 # ERROR_FILE_NOT_FOUND + try: + proc.wait(1) + except subprocess.TimeoutExpired: + proc.terminate() + + +@os_helper.skip_unless_symlink +>>>>>>> 39e6b8ae6a (bpo-46785: Fix race condition between os.stat() and unlink on Windows (GH-31858)) class NonLocalSymlinkTests(unittest.TestCase): def setUp(self): diff --git a/Misc/ACKS b/Misc/ACKS index d6b9650f642a96..a9f15b4f967267 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -1664,6 +1664,7 @@ Anthony Starks David Steele Oliver Steele Greg Stein +Itai Steinherz Marek Stepniowski Baruch Sterin Chris Stern diff --git a/Misc/NEWS.d/next/Windows/2022-03-13-20-35-41.bpo-46785.Pnknyl.rst b/Misc/NEWS.d/next/Windows/2022-03-13-20-35-41.bpo-46785.Pnknyl.rst new file mode 100644 index 00000000000000..0a87abd77c8ffe --- /dev/null +++ b/Misc/NEWS.d/next/Windows/2022-03-13-20-35-41.bpo-46785.Pnknyl.rst @@ -0,0 +1 @@ +Fix race condition between :func:`os.stat` and unlinking a file on Windows, by using errors codes returned by ``FindFirstFileW()`` when appropriate in ``win32_xstat_impl``. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 29d61268e1b5da..1270af735e2b2c 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -1853,7 +1853,17 @@ win32_xstat_impl(const wchar_t *path, struct _Py_stat_struct *result, /* Try reading the parent directory. */ if (!attributes_from_dir(path, &fileInfo, &tagInfo.ReparseTag)) { /* Cannot read the parent directory. */ - SetLastError(error); + switch (GetLastError()) { + case ERROR_FILE_NOT_FOUND: /* File cannot be found */ + case ERROR_PATH_NOT_FOUND: /* File parent directory cannot be found */ + case ERROR_NOT_READY: /* Drive exists but unavailable */ + case ERROR_BAD_NET_NAME: /* Remote drive unavailable */ + break; + /* Restore the error from CreateFileW(). */ + default: + SetLastError(error); + } + return -1; } if (fileInfo.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) { From 26cc8454eeddf872115e11ac50bd3245b9e631b6 Mon Sep 17 00:00:00 2001 From: Itai Steinherz Date: Tue, 10 May 2022 00:21:25 +0300 Subject: [PATCH 2/2] Fixup merge --- Lib/test/test_os.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 07499b7e5b04ab..78dd3151b36f92 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -2715,10 +2715,6 @@ def test_getfinalpathname_handles(self): self.assertEqual(0, handle_delta) -<<<<<<< HEAD -@support.skip_unless_symlink -======= - @support.requires_subprocess() def test_stat_unlink_race(self): # bpo-46785: the implementation of os.stat() falls back to reading # the parent directory if CreateFileW() fails with a permission @@ -2726,8 +2722,8 @@ def test_stat_unlink_race(self): # directory are subsequently unlinked, or because the volume or # share are no longer available, then the original permission error # should not be restored. - filename = os_helper.TESTFN - self.addCleanup(os_helper.unlink, filename) + filename = support.TESTFN + self.addCleanup(support.unlink, filename) deadline = time.time() + 5 command = textwrap.dedent("""\ import os @@ -2761,8 +2757,7 @@ def test_stat_unlink_race(self): proc.terminate() -@os_helper.skip_unless_symlink ->>>>>>> 39e6b8ae6a (bpo-46785: Fix race condition between os.stat() and unlink on Windows (GH-31858)) +@support.skip_unless_symlink class NonLocalSymlinkTests(unittest.TestCase): def setUp(self):