From 4ff124345f4d79fe5b6fb1fbf92209edb3b774fa Mon Sep 17 00:00:00 2001 From: Timofey Ivankov Date: Wed, 8 Jul 2026 22:02:25 +0300 Subject: [PATCH 1/5] gh-152754: Fix crash when an os.scandir iterator is shared between threads Co-authored-by: Neil Schemenauer --- Lib/test/test_free_threading/test_os.py | 76 ++++++++ ...-07-08-22-01-04.gh-issue-152754.3fW5kf.rst | 2 + Modules/posixmodule.c | 175 ++++++++++++------ 3 files changed, 197 insertions(+), 56 deletions(-) create mode 100644 Lib/test/test_free_threading/test_os.py create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-07-08-22-01-04.gh-issue-152754.3fW5kf.rst diff --git a/Lib/test/test_free_threading/test_os.py b/Lib/test/test_free_threading/test_os.py new file mode 100644 index 000000000000000..96a23edc0d1319a --- /dev/null +++ b/Lib/test/test_free_threading/test_os.py @@ -0,0 +1,76 @@ +import os +import shutil +import tempfile +import threading +import unittest + +from test import support +from test.support import threading_helper + + +if support.check_sanitizer(thread=True): + NUMITEMS = 200 + N_NEXT = 2 + N_CLOSE = 2 + REPEAT = 10 +else: + NUMITEMS = 1000 + N_NEXT = 6 + N_CLOSE = 3 + REPEAT = 20 + + +@threading_helper.requires_working_threading() +class ScandirThreadingTest(unittest.TestCase): + def setUp(self): + self.dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.dir, ignore_errors=True) + self.names = set() + for i in range(NUMITEMS): + name = f"f{i}" + with open(os.path.join(self.dir, name), "w"): + pass + self.names.add(name) + + def run_threads(self, funcs): + threading_helper.run_concurrently(funcs) + + def test_close_racing_next(self): + # gh-152754: one thread's next() racing another's close() must not crash. + def nexter(): + for _ in self.it: + pass + + def closer(): + self.it.close() + + for _ in range(REPEAT): + self.it = os.scandir(self.dir) + try: + self.run_threads([nexter] * N_NEXT + [closer] * N_CLOSE) + finally: + self.it.close() + + def test_shared_next(self): + # gh-152754: threads sharing one iterator must not crash or lose entries. + self.it = os.scandir(self.dir) + results = [] + results_lock = threading.Lock() + + def worker(): + local = [] + for entry in self.it: + local.append(entry.name) + with results_lock: + results.extend(local) + + try: + self.run_threads([worker] * (N_NEXT + N_CLOSE)) + finally: + self.it.close() + + self.assertEqual(sorted(results), sorted(self.names)) + + +if __name__ == "__main__": + unittest.main() diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-07-08-22-01-04.gh-issue-152754.3fW5kf.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-07-08-22-01-04.gh-issue-152754.3fW5kf.rst new file mode 100644 index 000000000000000..c62d92bccbf8caf --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-07-08-22-01-04.gh-issue-152754.3fW5kf.rst @@ -0,0 +1,2 @@ +Fix a crash when the same :func:`os.scandir` iterator is used concurrently +from multiple threads on the :term:`free-threaded ` build. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 57db175336702e2..f063861bfe39b5f 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -16821,6 +16821,11 @@ DirEntry_from_posix_info(PyObject *module, path_t *path, const char *name, typedef struct { PyObject_HEAD +#ifdef Py_GIL_DISABLED + // Protects scandir iterator state when a os.scandir() iterator is used + // from multiple threads. + PyMutex mutex; +#endif path_t path; #ifdef MS_WINDOWS HANDLE handle; @@ -16836,67 +16841,89 @@ typedef struct { #define ScandirIterator_CAST(op) ((ScandirIterator *)(op)) +#ifdef Py_GIL_DISABLED +# define SCANDIR_ITERATOR_LOCK(iterator) PyMutex_Lock(&(iterator)->mutex) +# define SCANDIR_ITERATOR_UNLOCK(iterator) PyMutex_Unlock(&(iterator)->mutex) +#else +# define SCANDIR_ITERATOR_LOCK(iterator) ((void)0) +# define SCANDIR_ITERATOR_UNLOCK(iterator) ((void)0) +#endif + #ifdef MS_WINDOWS static int ScandirIterator_is_closed(ScandirIterator *iterator) { - return iterator->handle == INVALID_HANDLE_VALUE; + SCANDIR_ITERATOR_LOCK(iterator); + int closed = iterator->handle == INVALID_HANDLE_VALUE; + SCANDIR_ITERATOR_UNLOCK(iterator); + return closed; } static void ScandirIterator_closedir(ScandirIterator *iterator) { + SCANDIR_ITERATOR_LOCK(iterator); HANDLE handle = iterator->handle; - - if (handle == INVALID_HANDLE_VALUE) - return; - iterator->handle = INVALID_HANDLE_VALUE; - Py_BEGIN_ALLOW_THREADS - FindClose(handle); - Py_END_ALLOW_THREADS + SCANDIR_ITERATOR_UNLOCK(iterator); + + if (handle != INVALID_HANDLE_VALUE) { + Py_BEGIN_ALLOW_THREADS + FindClose(handle); + Py_END_ALLOW_THREADS + } } static PyObject * ScandirIterator_iternext(PyObject *op) { ScandirIterator *iterator = ScandirIterator_CAST(op); - WIN32_FIND_DATAW *file_data = &iterator->file_data; + WIN32_FIND_DATAW file_data; BOOL success; - PyObject *entry; + DWORD error = ERROR_SUCCESS; + int found = 0; + SCANDIR_ITERATOR_LOCK(iterator); /* Happens if the iterator is iterated twice, or closed explicitly */ - if (iterator->handle == INVALID_HANDLE_VALUE) - return NULL; - - while (1) { + while (iterator->handle != INVALID_HANDLE_VALUE) { if (!iterator->first_time) { Py_BEGIN_ALLOW_THREADS - success = FindNextFileW(iterator->handle, file_data); + success = FindNextFileW(iterator->handle, &iterator->file_data); + if (!success) { + error = GetLastError(); + } Py_END_ALLOW_THREADS if (!success) { - /* Error or no more files */ - if (GetLastError() != ERROR_NO_MORE_FILES) - path_error(&iterator->path); break; } } iterator->first_time = 0; /* Skip over . and .. */ - if (wcscmp(file_data->cFileName, L".") != 0 && - wcscmp(file_data->cFileName, L"..") != 0) + if (wcscmp(iterator->file_data.cFileName, L".") != 0 && + wcscmp(iterator->file_data.cFileName, L"..") != 0) { - PyObject *module = PyType_GetModule(Py_TYPE(iterator)); - entry = DirEntry_from_find_data(module, &iterator->path, file_data); - if (!entry) - break; - return entry; + file_data = iterator->file_data; + found = 1; + break; } /* Loop till we get a non-dot directory or finish iterating */ } + SCANDIR_ITERATOR_UNLOCK(iterator); + + if (found) { + PyObject *module = PyType_GetModule(Py_TYPE(iterator)); + PyObject *entry = DirEntry_from_find_data(module, &iterator->path, &file_data); + if (entry != NULL) { + return entry; + } + } + else if (error != ERROR_SUCCESS && error != ERROR_NO_MORE_FILES) { + SetLastError(error); + path_error(&iterator->path); + } /* Error or no more files */ ScandirIterator_closedir(iterator); @@ -16908,27 +16935,30 @@ ScandirIterator_iternext(PyObject *op) static int ScandirIterator_is_closed(ScandirIterator *iterator) { - return !iterator->dirp; + SCANDIR_ITERATOR_LOCK(iterator); + int closed = iterator->dirp == NULL; + SCANDIR_ITERATOR_UNLOCK(iterator); + return closed; } static void ScandirIterator_closedir(ScandirIterator *iterator) { + SCANDIR_ITERATOR_LOCK(iterator); DIR *dirp = iterator->dirp; - - if (!dirp) - return; - iterator->dirp = NULL; - Py_BEGIN_ALLOW_THREADS + SCANDIR_ITERATOR_UNLOCK(iterator); + + if (dirp != NULL) { + Py_BEGIN_ALLOW_THREADS #ifdef HAVE_FDOPENDIR - if (iterator->path.is_fd) { - rewinddir(dirp); - } + if (iterator->path.is_fd) { + rewinddir(dirp); + } #endif - closedir(dirp); - Py_END_ALLOW_THREADS - return; + closedir(dirp); + Py_END_ALLOW_THREADS + } } static PyObject * @@ -16936,24 +16966,29 @@ ScandirIterator_iternext(PyObject *op) { ScandirIterator *iterator = ScandirIterator_CAST(op); struct dirent *direntp; - Py_ssize_t name_len; + Py_ssize_t name_len = 0; int is_dot; - PyObject *entry; + int found = 0; + int error = 0; + int no_memory = 0; + char *name = NULL; + ino_t d_ino = 0; +#ifdef HAVE_DIRENT_D_TYPE + unsigned char d_type = 0; +#endif + SCANDIR_ITERATOR_LOCK(iterator); /* Happens if the iterator is iterated twice, or closed explicitly */ - if (!iterator->dirp) - return NULL; - - while (1) { - errno = 0; + while (iterator->dirp != NULL) { Py_BEGIN_ALLOW_THREADS + errno = 0; direntp = readdir(iterator->dirp); + if (direntp == NULL) { + error = errno; + } Py_END_ALLOW_THREADS if (!direntp) { - /* Error or no more files */ - if (errno != 0) - path_error(&iterator->path); break; } @@ -16962,21 +16997,46 @@ ScandirIterator_iternext(PyObject *op) is_dot = direntp->d_name[0] == '.' && (name_len == 1 || (direntp->d_name[1] == '.' && name_len == 2)); if (!is_dot) { - PyObject *module = PyType_GetModule(Py_TYPE(iterator)); - entry = DirEntry_from_posix_info(module, - &iterator->path, direntp->d_name, - name_len, direntp->d_ino + name = PyMem_RawMalloc(name_len + 1); + if (name == NULL) { + no_memory = 1; + break; + } + memcpy(name, direntp->d_name, name_len); + name[name_len] = '\0'; + d_ino = direntp->d_ino; #ifdef HAVE_DIRENT_D_TYPE - , direntp->d_type + d_type = direntp->d_type; #endif - ); - if (!entry) - break; - return entry; + found = 1; + break; } /* Loop till we get a non-dot directory or finish iterating */ } + SCANDIR_ITERATOR_UNLOCK(iterator); + + if (found) { + PyObject *module = PyType_GetModule(Py_TYPE(iterator)); + PyObject *entry = DirEntry_from_posix_info(module, + &iterator->path, name, + name_len, d_ino +#ifdef HAVE_DIRENT_D_TYPE + , d_type +#endif + ); + PyMem_RawFree(name); + if (entry != NULL) { + return entry; + } + } + else if (no_memory) { + PyErr_NoMemory(); + } + else if (error != 0) { + errno = error; + path_error(&iterator->path); + } /* Error or no more files */ ScandirIterator_closedir(iterator); @@ -17114,6 +17174,9 @@ os_scandir_impl(PyObject *module, path_t *path) if (!iterator) return NULL; +#ifdef Py_GIL_DISABLED + iterator->mutex = (PyMutex){0}; +#endif #ifdef MS_WINDOWS iterator->handle = INVALID_HANDLE_VALUE; #else From 4360e83c32c079da4fd91706af54eddd4550714e Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Thu, 9 Jul 2026 14:56:28 -0700 Subject: [PATCH 2/5] Put news file into Library section. --- .../2026-07-09-14-56-24.gh-issue-152754.CyyC5j.rst} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename Misc/NEWS.d/next/{Core_and_Builtins/2026-07-08-22-01-04.gh-issue-152754.3fW5kf.rst => Library/2026-07-09-14-56-24.gh-issue-152754.CyyC5j.rst} (100%) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-07-08-22-01-04.gh-issue-152754.3fW5kf.rst b/Misc/NEWS.d/next/Library/2026-07-09-14-56-24.gh-issue-152754.CyyC5j.rst similarity index 100% rename from Misc/NEWS.d/next/Core_and_Builtins/2026-07-08-22-01-04.gh-issue-152754.3fW5kf.rst rename to Misc/NEWS.d/next/Library/2026-07-09-14-56-24.gh-issue-152754.CyyC5j.rst From 84219b42732f769cd43f122fc97914095ed866d7 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Wed, 15 Jul 2026 18:01:54 -0700 Subject: [PATCH 3/5] Use FT_MUTEX_LOCK/UNLOCK. --- Modules/posixmodule.c | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index f063861bfe39b5f..df36268febb41f4 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -26,6 +26,7 @@ #include "pycore_object.h" // _PyObject_LookupSpecial() #include "pycore_pylifecycle.h" // _PyOS_URandom() #include "pycore_pystate.h" // _PyInterpreterState_GET() +#include "pycore_pyatomic_ft_wrappers.h" // FT_MUTEX_LOCK() #include "pycore_signal.h" // Py_NSIG #include "pycore_time.h" // _PyLong_FromTime_t() #include "pycore_tuple.h" // _PyTuple_FromPairSteal @@ -16841,32 +16842,24 @@ typedef struct { #define ScandirIterator_CAST(op) ((ScandirIterator *)(op)) -#ifdef Py_GIL_DISABLED -# define SCANDIR_ITERATOR_LOCK(iterator) PyMutex_Lock(&(iterator)->mutex) -# define SCANDIR_ITERATOR_UNLOCK(iterator) PyMutex_Unlock(&(iterator)->mutex) -#else -# define SCANDIR_ITERATOR_LOCK(iterator) ((void)0) -# define SCANDIR_ITERATOR_UNLOCK(iterator) ((void)0) -#endif - #ifdef MS_WINDOWS static int ScandirIterator_is_closed(ScandirIterator *iterator) { - SCANDIR_ITERATOR_LOCK(iterator); + FT_MUTEX_LOCK(&iterator->mutex); int closed = iterator->handle == INVALID_HANDLE_VALUE; - SCANDIR_ITERATOR_UNLOCK(iterator); + FT_MUTEX_UNLOCK(&iterator->mutex); return closed; } static void ScandirIterator_closedir(ScandirIterator *iterator) { - SCANDIR_ITERATOR_LOCK(iterator); + FT_MUTEX_LOCK(&iterator->mutex); HANDLE handle = iterator->handle; iterator->handle = INVALID_HANDLE_VALUE; - SCANDIR_ITERATOR_UNLOCK(iterator); + FT_MUTEX_UNLOCK(&iterator->mutex); if (handle != INVALID_HANDLE_VALUE) { Py_BEGIN_ALLOW_THREADS @@ -16884,7 +16877,7 @@ ScandirIterator_iternext(PyObject *op) DWORD error = ERROR_SUCCESS; int found = 0; - SCANDIR_ITERATOR_LOCK(iterator); + FT_MUTEX_LOCK(&iterator->mutex); /* Happens if the iterator is iterated twice, or closed explicitly */ while (iterator->handle != INVALID_HANDLE_VALUE) { if (!iterator->first_time) { @@ -16911,7 +16904,7 @@ ScandirIterator_iternext(PyObject *op) /* Loop till we get a non-dot directory or finish iterating */ } - SCANDIR_ITERATOR_UNLOCK(iterator); + FT_MUTEX_UNLOCK(&iterator->mutex); if (found) { PyObject *module = PyType_GetModule(Py_TYPE(iterator)); @@ -16935,19 +16928,19 @@ ScandirIterator_iternext(PyObject *op) static int ScandirIterator_is_closed(ScandirIterator *iterator) { - SCANDIR_ITERATOR_LOCK(iterator); + FT_MUTEX_LOCK(&iterator->mutex); int closed = iterator->dirp == NULL; - SCANDIR_ITERATOR_UNLOCK(iterator); + FT_MUTEX_UNLOCK(&iterator->mutex); return closed; } static void ScandirIterator_closedir(ScandirIterator *iterator) { - SCANDIR_ITERATOR_LOCK(iterator); + FT_MUTEX_LOCK(&iterator->mutex); DIR *dirp = iterator->dirp; iterator->dirp = NULL; - SCANDIR_ITERATOR_UNLOCK(iterator); + FT_MUTEX_UNLOCK(&iterator->mutex); if (dirp != NULL) { Py_BEGIN_ALLOW_THREADS @@ -16977,7 +16970,7 @@ ScandirIterator_iternext(PyObject *op) unsigned char d_type = 0; #endif - SCANDIR_ITERATOR_LOCK(iterator); + FT_MUTEX_LOCK(&iterator->mutex); /* Happens if the iterator is iterated twice, or closed explicitly */ while (iterator->dirp != NULL) { Py_BEGIN_ALLOW_THREADS @@ -17014,7 +17007,7 @@ ScandirIterator_iternext(PyObject *op) /* Loop till we get a non-dot directory or finish iterating */ } - SCANDIR_ITERATOR_UNLOCK(iterator); + FT_MUTEX_UNLOCK(&iterator->mutex); if (found) { PyObject *module = PyType_GetModule(Py_TYPE(iterator)); From 00267a282506d8cb994e0c1887f6491b34efe75b Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Wed, 12 Aug 2026 10:38:27 -0700 Subject: [PATCH 4/5] Use mutex on GIL-enabled builds too. This race is not free-threading specific. --- Lib/test/test_free_threading/test_os.py | 76 ------------------- Lib/test/test_os/test_os.py | 68 +++++++++++++++++ ...-07-09-14-56-24.gh-issue-152754.CyyC5j.rst | 4 +- Modules/posixmodule.c | 36 ++++----- 4 files changed, 87 insertions(+), 97 deletions(-) delete mode 100644 Lib/test/test_free_threading/test_os.py diff --git a/Lib/test/test_free_threading/test_os.py b/Lib/test/test_free_threading/test_os.py deleted file mode 100644 index 96a23edc0d1319a..000000000000000 --- a/Lib/test/test_free_threading/test_os.py +++ /dev/null @@ -1,76 +0,0 @@ -import os -import shutil -import tempfile -import threading -import unittest - -from test import support -from test.support import threading_helper - - -if support.check_sanitizer(thread=True): - NUMITEMS = 200 - N_NEXT = 2 - N_CLOSE = 2 - REPEAT = 10 -else: - NUMITEMS = 1000 - N_NEXT = 6 - N_CLOSE = 3 - REPEAT = 20 - - -@threading_helper.requires_working_threading() -class ScandirThreadingTest(unittest.TestCase): - def setUp(self): - self.dir = tempfile.mkdtemp() - self.addCleanup(shutil.rmtree, self.dir, ignore_errors=True) - self.names = set() - for i in range(NUMITEMS): - name = f"f{i}" - with open(os.path.join(self.dir, name), "w"): - pass - self.names.add(name) - - def run_threads(self, funcs): - threading_helper.run_concurrently(funcs) - - def test_close_racing_next(self): - # gh-152754: one thread's next() racing another's close() must not crash. - def nexter(): - for _ in self.it: - pass - - def closer(): - self.it.close() - - for _ in range(REPEAT): - self.it = os.scandir(self.dir) - try: - self.run_threads([nexter] * N_NEXT + [closer] * N_CLOSE) - finally: - self.it.close() - - def test_shared_next(self): - # gh-152754: threads sharing one iterator must not crash or lose entries. - self.it = os.scandir(self.dir) - results = [] - results_lock = threading.Lock() - - def worker(): - local = [] - for entry in self.it: - local.append(entry.name) - with results_lock: - results.extend(local) - - try: - self.run_threads([worker] * (N_NEXT + N_CLOSE)) - finally: - self.it.close() - - self.assertEqual(sorted(results), sorted(self.names)) - - -if __name__ == "__main__": - unittest.main() diff --git a/Lib/test/test_os/test_os.py b/Lib/test/test_os/test_os.py index 7a49cfa0c29ec5b..cc9a653fffc760a 100644 --- a/Lib/test/test_os/test_os.py +++ b/Lib/test/test_os/test_os.py @@ -24,6 +24,7 @@ import sysconfig import tempfile import textwrap +import threading import time import types import unittest @@ -35,6 +36,7 @@ from test.support import infinite_recursion from test.support import requires_root_user from test.support import requires_non_root_user +from test.support import threading_helper from test.support import warnings_helper from platform import win32_is_iot from .utils import create_file @@ -5351,6 +5353,72 @@ def test_resource_warning(self): del iterator +@threading_helper.requires_working_threading() +class ScandirThreadingTest(unittest.TestCase): + # gh-152754: an os.scandir() iterator shared between threads must not crash. + + if support.check_sanitizer(thread=True): + SCANDIR_NUMITEMS = 200 + SCANDIR_N_NEXT = 2 + SCANDIR_N_CLOSE = 2 + SCANDIR_REPEAT = 10 + else: + SCANDIR_NUMITEMS = 1000 + SCANDIR_N_NEXT = 6 + SCANDIR_N_CLOSE = 3 + SCANDIR_REPEAT = 20 + + def setUp(self): + self.dir = os.path.realpath(os_helper.TESTFN) + self.addCleanup(os_helper.rmtree, self.dir) + os.mkdir(self.dir) + self.names = set() + for i in range(self.SCANDIR_NUMITEMS): + name = f"f{i}" + create_file(os.path.join(self.dir, name)) + self.names.add(name) + + def test_close_racing_next(self): + # One thread's next() racing another's close() must not crash. + def nexter(): + for _ in self.it: + pass + + def closer(): + self.it.close() + + funcs = [nexter] * self.SCANDIR_N_NEXT + [closer] * self.SCANDIR_N_CLOSE + for _ in range(self.SCANDIR_REPEAT): + self.it = os.scandir(self.dir) + try: + threading_helper.run_concurrently(funcs) + finally: + self.it.close() + + def test_shared_next(self): + # Threads sharing one iterator must not crash or lose entries: every + # entry must be handed to exactly one thread. + expected = sorted(self.names) + nthreads = self.SCANDIR_N_NEXT + self.SCANDIR_N_CLOSE + + for _ in range(self.SCANDIR_REPEAT): + self.it = os.scandir(self.dir) + results = [] + results_lock = threading.Lock() + + def worker(): + local = [entry.name for entry in self.it] + with results_lock: + results.extend(local) + + try: + threading_helper.run_concurrently([worker] * nthreads) + finally: + self.it.close() + + self.assertEqual(sorted(results), expected) + + class TestPEP519(unittest.TestCase): # Abstracted so it can be overridden to test pure Python implementation diff --git a/Misc/NEWS.d/next/Library/2026-07-09-14-56-24.gh-issue-152754.CyyC5j.rst b/Misc/NEWS.d/next/Library/2026-07-09-14-56-24.gh-issue-152754.CyyC5j.rst index c62d92bccbf8caf..7bdb8399e3ab49e 100644 --- a/Misc/NEWS.d/next/Library/2026-07-09-14-56-24.gh-issue-152754.CyyC5j.rst +++ b/Misc/NEWS.d/next/Library/2026-07-09-14-56-24.gh-issue-152754.CyyC5j.rst @@ -1,2 +1,4 @@ Fix a crash when the same :func:`os.scandir` iterator is used concurrently -from multiple threads on the :term:`free-threaded ` build. +from multiple threads. Iterating and closing the iterator are now serialized +with a per-iterator lock, so that closing it no longer releases the +directory handle while another thread is still reading from it. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 88767422520ccb1..0ff16a8e4b60030 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -24,9 +24,9 @@ #include "pycore_long.h" // _PyLong_IsNegative() #include "pycore_moduleobject.h" // _PyModule_GetState() #include "pycore_object.h" // _PyObject_LookupSpecial() +#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_INT_RELAXED() #include "pycore_pylifecycle.h" // _PyOS_URandom() #include "pycore_pystate.h" // _PyInterpreterState_GET() -#include "pycore_pyatomic_ft_wrappers.h" // FT_MUTEX_LOCK() #include "pycore_signal.h" // Py_NSIG #include "pycore_time.h" // _PyLong_FromTime_t() #include "pycore_tuple.h" // _PyTuple_FromPairSteal @@ -16941,11 +16941,6 @@ DirEntry_from_posix_info(PyObject *module, path_t *path, const char *name, typedef struct { PyObject_HEAD -#ifdef Py_GIL_DISABLED - // Protects scandir iterator state when a os.scandir() iterator is used - // from multiple threads. - PyMutex mutex; -#endif path_t path; #ifdef MS_WINDOWS HANDLE handle; @@ -16957,6 +16952,9 @@ typedef struct { #ifdef HAVE_FDOPENDIR int fd; #endif + // Protects the iterator state when an os.scandir() iterator is used from + // multiple threads. + PyMutex mutex; } ScandirIterator; #define ScandirIterator_CAST(op) ((ScandirIterator *)(op)) @@ -16966,19 +16964,19 @@ typedef struct { static int ScandirIterator_is_closed(ScandirIterator *iterator) { - FT_MUTEX_LOCK(&iterator->mutex); + PyMutex_Lock(&iterator->mutex); int closed = iterator->handle == INVALID_HANDLE_VALUE; - FT_MUTEX_UNLOCK(&iterator->mutex); + PyMutex_Unlock(&iterator->mutex); return closed; } static void ScandirIterator_closedir(ScandirIterator *iterator) { - FT_MUTEX_LOCK(&iterator->mutex); + PyMutex_Lock(&iterator->mutex); HANDLE handle = iterator->handle; iterator->handle = INVALID_HANDLE_VALUE; - FT_MUTEX_UNLOCK(&iterator->mutex); + PyMutex_Unlock(&iterator->mutex); if (handle != INVALID_HANDLE_VALUE) { Py_BEGIN_ALLOW_THREADS @@ -16996,7 +16994,7 @@ ScandirIterator_iternext(PyObject *op) DWORD error = ERROR_SUCCESS; int found = 0; - FT_MUTEX_LOCK(&iterator->mutex); + PyMutex_Lock(&iterator->mutex); /* Happens if the iterator is iterated twice, or closed explicitly */ while (iterator->handle != INVALID_HANDLE_VALUE) { if (!iterator->first_time) { @@ -17023,7 +17021,7 @@ ScandirIterator_iternext(PyObject *op) /* Loop till we get a non-dot directory or finish iterating */ } - FT_MUTEX_UNLOCK(&iterator->mutex); + PyMutex_Unlock(&iterator->mutex); if (found) { PyObject *module = PyType_GetModule(Py_TYPE(iterator)); @@ -17047,19 +17045,19 @@ ScandirIterator_iternext(PyObject *op) static int ScandirIterator_is_closed(ScandirIterator *iterator) { - FT_MUTEX_LOCK(&iterator->mutex); + PyMutex_Lock(&iterator->mutex); int closed = iterator->dirp == NULL; - FT_MUTEX_UNLOCK(&iterator->mutex); + PyMutex_Unlock(&iterator->mutex); return closed; } static void ScandirIterator_closedir(ScandirIterator *iterator) { - FT_MUTEX_LOCK(&iterator->mutex); + PyMutex_Lock(&iterator->mutex); DIR *dirp = iterator->dirp; iterator->dirp = NULL; - FT_MUTEX_UNLOCK(&iterator->mutex); + PyMutex_Unlock(&iterator->mutex); if (dirp != NULL) { Py_BEGIN_ALLOW_THREADS @@ -17089,7 +17087,7 @@ ScandirIterator_iternext(PyObject *op) unsigned char d_type = 0; #endif - FT_MUTEX_LOCK(&iterator->mutex); + PyMutex_Lock(&iterator->mutex); /* Happens if the iterator is iterated twice, or closed explicitly */ while (iterator->dirp != NULL) { Py_BEGIN_ALLOW_THREADS @@ -17126,7 +17124,7 @@ ScandirIterator_iternext(PyObject *op) /* Loop till we get a non-dot directory or finish iterating */ } - FT_MUTEX_UNLOCK(&iterator->mutex); + PyMutex_Unlock(&iterator->mutex); if (found) { PyObject *module = PyType_GetModule(Py_TYPE(iterator)); @@ -17286,9 +17284,7 @@ os_scandir_impl(PyObject *module, path_t *path) if (!iterator) return NULL; -#ifdef Py_GIL_DISABLED iterator->mutex = (PyMutex){0}; -#endif #ifdef MS_WINDOWS iterator->handle = INVALID_HANDLE_VALUE; #else From 854b5b983706db2e61165d3df96b52b8d370a09b Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Wed, 12 Aug 2026 12:42:55 -0700 Subject: [PATCH 5/5] Revise so close() doesn't wait for mutex. This makes it not safe to share iterators between multiple threads so document that. --- Doc/library/os.rst | 5 + Lib/test/test_os/test_os.py | 12 +++ ...-07-09-14-56-24.gh-issue-152754.CyyC5j.rst | 7 +- Modules/posixmodule.c | 99 ++++++++++++------- 4 files changed, 85 insertions(+), 38 deletions(-) diff --git a/Doc/library/os.rst b/Doc/library/os.rst index fceadde7df6bf48..b1055782c7d0c3e 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -2956,6 +2956,11 @@ features: .. audit-event:: os.scandir path os.scandir + Sharing a :func:`scandir` iterator between threads will not corrupt the + iterator, but it is subject to :term:`race conditions `: + which entries each thread receives is unspecified, and closing the iterator + while another thread is iterating ends that iteration early. + The :func:`scandir` iterator supports the :term:`context manager` protocol and has the following method: diff --git a/Lib/test/test_os/test_os.py b/Lib/test/test_os/test_os.py index cc9a653fffc760a..81b3043eb7e75bc 100644 --- a/Lib/test/test_os/test_os.py +++ b/Lib/test/test_os/test_os.py @@ -5352,6 +5352,18 @@ def test_resource_warning(self): with self.check_no_resource_warning(): del iterator + def test_no_resource_warning_when_open_fails(self): + # gh-152754: a scandir() call that never opened a directory owns + # nothing, and must not report an unclosed iterator. + self.create_file("file.txt") + missing = os.path.join(self.path, "missing") + not_a_dir = os.path.join(self.path, "file.txt") + for path in (missing, not_a_dir): + with self.subTest(path=path): + with self.check_no_resource_warning(): + with self.assertRaises(OSError): + os.scandir(path) + @threading_helper.requires_working_threading() class ScandirThreadingTest(unittest.TestCase): diff --git a/Misc/NEWS.d/next/Library/2026-07-09-14-56-24.gh-issue-152754.CyyC5j.rst b/Misc/NEWS.d/next/Library/2026-07-09-14-56-24.gh-issue-152754.CyyC5j.rst index 7bdb8399e3ab49e..b0378d9afe103e7 100644 --- a/Misc/NEWS.d/next/Library/2026-07-09-14-56-24.gh-issue-152754.CyyC5j.rst +++ b/Misc/NEWS.d/next/Library/2026-07-09-14-56-24.gh-issue-152754.CyyC5j.rst @@ -1,4 +1,5 @@ Fix a crash when the same :func:`os.scandir` iterator is used concurrently -from multiple threads. Iterating and closing the iterator are now serialized -with a per-iterator lock, so that closing it no longer releases the -directory handle while another thread is still reading from it. +from multiple threads. It no longer releases the directory handle while +another thread is reading from it. Sharing an iterator between threads +remains subject to race conditions: which entries each thread receives is +unspecified. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 0ff16a8e4b60030..38b0bcda27da497 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -21,10 +21,10 @@ #include "pycore_import.h" // _PyImport_AcquireLock() #include "pycore_initconfig.h" // _PyStatus_EXCEPTION() #include "pycore_jit_unwind.h" // _Py_jit_debug_mutex +#include "pycore_lock.h" // _PyMutex_LockTimed() #include "pycore_long.h" // _PyLong_IsNegative() #include "pycore_moduleobject.h" // _PyModule_GetState() #include "pycore_object.h" // _PyObject_LookupSpecial() -#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_INT_RELAXED() #include "pycore_pylifecycle.h" // _PyOS_URandom() #include "pycore_pystate.h" // _PyInterpreterState_GET() #include "pycore_signal.h" // Py_NSIG @@ -16952,9 +16952,13 @@ typedef struct { #ifdef HAVE_FDOPENDIR int fd; #endif - // Protects the iterator state when an os.scandir() iterator is used from - // multiple threads. - PyMutex mutex; + // Sharing the iterator between threads is subject to race conditions: + // which entries each thread receives is unspecified. It must not + // corrupt the iterator or crash. Since we don't want close() to be + // held up by a blocking directory read, we set the 'closed' flag if + // there are reads in progress. + PyMutex read_mutex; + uint8_t closed; } ScandirIterator; #define ScandirIterator_CAST(op) ((ScandirIterator *)(op)) @@ -16964,19 +16968,21 @@ typedef struct { static int ScandirIterator_is_closed(ScandirIterator *iterator) { - PyMutex_Lock(&iterator->mutex); - int closed = iterator->handle == INVALID_HANDLE_VALUE; - PyMutex_Unlock(&iterator->mutex); - return closed; + return _Py_atomic_load_uint8(&iterator->closed); } static void ScandirIterator_closedir(ScandirIterator *iterator) { - PyMutex_Lock(&iterator->mutex); - HANDLE handle = iterator->handle; - iterator->handle = INVALID_HANDLE_VALUE; - PyMutex_Unlock(&iterator->mutex); + HANDLE handle = INVALID_HANDLE_VALUE; + + _Py_atomic_store_uint8(&iterator->closed, 1); + if (_PyMutex_LockTimed(&iterator->read_mutex, 0, 0) == PY_LOCK_ACQUIRED) { + // no reads in progress, we can close the handle + handle = iterator->handle; + iterator->handle = INVALID_HANDLE_VALUE; + PyMutex_Unlock(&iterator->read_mutex); + } if (handle != INVALID_HANDLE_VALUE) { Py_BEGIN_ALLOW_THREADS @@ -16994,9 +17000,11 @@ ScandirIterator_iternext(PyObject *op) DWORD error = ERROR_SUCCESS; int found = 0; - PyMutex_Lock(&iterator->mutex); + PyMutex_Lock(&iterator->read_mutex); /* Happens if the iterator is iterated twice, or closed explicitly */ - while (iterator->handle != INVALID_HANDLE_VALUE) { + while (iterator->handle != INVALID_HANDLE_VALUE && + !_Py_atomic_load_uint8_relaxed(&iterator->closed)) + { if (!iterator->first_time) { Py_BEGIN_ALLOW_THREADS success = FindNextFileW(iterator->handle, &iterator->file_data); @@ -17021,7 +17029,11 @@ ScandirIterator_iternext(PyObject *op) /* Loop till we get a non-dot directory or finish iterating */ } - PyMutex_Unlock(&iterator->mutex); + PyMutex_Unlock(&iterator->read_mutex); + + if (found && ScandirIterator_is_closed(iterator)) { + ScandirIterator_closedir(iterator); // deferred close + } if (found) { PyObject *module = PyType_GetModule(Py_TYPE(iterator)); @@ -17045,19 +17057,21 @@ ScandirIterator_iternext(PyObject *op) static int ScandirIterator_is_closed(ScandirIterator *iterator) { - PyMutex_Lock(&iterator->mutex); - int closed = iterator->dirp == NULL; - PyMutex_Unlock(&iterator->mutex); - return closed; + return _Py_atomic_load_uint8(&iterator->closed); } static void ScandirIterator_closedir(ScandirIterator *iterator) { - PyMutex_Lock(&iterator->mutex); - DIR *dirp = iterator->dirp; - iterator->dirp = NULL; - PyMutex_Unlock(&iterator->mutex); + DIR *dirp = NULL; + + _Py_atomic_store_uint8(&iterator->closed, 1); + if (_PyMutex_LockTimed(&iterator->read_mutex, 0, 0) == PY_LOCK_ACQUIRED) { + // no reads in progress, we can close dirp + dirp = iterator->dirp; + iterator->dirp = NULL; + PyMutex_Unlock(&iterator->read_mutex); + } if (dirp != NULL) { Py_BEGIN_ALLOW_THREADS @@ -17081,15 +17095,18 @@ ScandirIterator_iternext(PyObject *op) int found = 0; int error = 0; int no_memory = 0; - char *name = NULL; + char namebuf[256]; + char *name = namebuf; ino_t d_ino = 0; #ifdef HAVE_DIRENT_D_TYPE unsigned char d_type = 0; #endif - PyMutex_Lock(&iterator->mutex); + PyMutex_Lock(&iterator->read_mutex); /* Happens if the iterator is iterated twice, or closed explicitly */ - while (iterator->dirp != NULL) { + while (iterator->dirp != NULL && + !_Py_atomic_load_uint8_relaxed(&iterator->closed)) + { Py_BEGIN_ALLOW_THREADS errno = 0; direntp = readdir(iterator->dirp); @@ -17107,10 +17124,12 @@ ScandirIterator_iternext(PyObject *op) is_dot = direntp->d_name[0] == '.' && (name_len == 1 || (direntp->d_name[1] == '.' && name_len == 2)); if (!is_dot) { - name = PyMem_RawMalloc(name_len + 1); - if (name == NULL) { - no_memory = 1; - break; + if ((size_t)name_len >= sizeof(namebuf)) { + name = PyMem_RawMalloc(name_len + 1); + if (name == NULL) { + no_memory = 1; + break; + } } memcpy(name, direntp->d_name, name_len); name[name_len] = '\0'; @@ -17124,7 +17143,11 @@ ScandirIterator_iternext(PyObject *op) /* Loop till we get a non-dot directory or finish iterating */ } - PyMutex_Unlock(&iterator->mutex); + PyMutex_Unlock(&iterator->read_mutex); + + if (found && ScandirIterator_is_closed(iterator)) { + ScandirIterator_closedir(iterator); // deferred close + } if (found) { PyObject *module = PyType_GetModule(Py_TYPE(iterator)); @@ -17135,7 +17158,9 @@ ScandirIterator_iternext(PyObject *op) , d_type #endif ); - PyMem_RawFree(name); + if (name != namebuf) { + PyMem_RawFree(name); + } if (entry != NULL) { return entry; } @@ -17184,9 +17209,11 @@ ScandirIterator_finalize(PyObject *op) /* Save the current exception, if any. */ PyObject *exc = PyErr_GetRaisedException(); - if (!ScandirIterator_is_closed(iterator)) { - ScandirIterator_closedir(iterator); + int was_closed = ScandirIterator_is_closed(iterator); + + ScandirIterator_closedir(iterator); + if (!was_closed) { if (PyErr_ResourceWarning(op, 1, "unclosed scandir iterator %R", iterator)) { @@ -17284,7 +17311,8 @@ os_scandir_impl(PyObject *module, path_t *path) if (!iterator) return NULL; - iterator->mutex = (PyMutex){0}; + iterator->read_mutex = (PyMutex){0}; + iterator->closed = 1; #ifdef MS_WINDOWS iterator->handle = INVALID_HANDLE_VALUE; #else @@ -17357,6 +17385,7 @@ os_scandir_impl(PyObject *module, path_t *path) } #endif + iterator->closed = 0; return (PyObject *)iterator; error: