https://github.com/python/cpython/commit/999a046b24cff4ba0e72b574196721f66bd08237 commit: 999a046b24cff4ba0e72b574196721f66bd08237 branch: main author: Neil Schemenauer <[email protected]> committer: nascheme <[email protected]> date: 2026-08-21T10:02:24-07:00 summary:
gh-152754: Fix crash when an os.scandir iterator is shared between threads (gh-153462) Co-authored-by: Timofey Ivankov <[email protected]> files: A Misc/NEWS.d/next/Library/2026-07-09-14-56-24.gh-issue-152754.CyyC5j.rst M Doc/library/os.rst M Lib/test/test_os/test_os.py M Modules/posixmodule.c diff --git a/Doc/library/os.rst b/Doc/library/os.rst index 525ec3a0c858ff1..0f033efefd209cd 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -2960,6 +2960,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 <race condition>`: + 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 7a49cfa0c29ec5b..81b3043eb7e75bc 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 @@ -5350,6 +5352,84 @@ 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): + # 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): 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 new file mode 100644 index 000000000000000..b0378d9afe103e7 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-07-09-14-56-24.gh-issue-152754.CyyC5j.rst @@ -0,0 +1,5 @@ +Fix a crash when the same :func:`os.scandir` iterator is used concurrently +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 d94291e2ae524bc..bec305a4042c49d 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -21,6 +21,7 @@ #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_LockFast() #include "pycore_long.h" // _PyLong_IsNegative() #include "pycore_moduleobject.h" // _PyModule_GetState() #include "pycore_object.h" // _PyObject_LookupSpecial() @@ -16985,6 +16986,13 @@ typedef struct { #ifdef HAVE_FDOPENDIR int fd; #endif + // 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)) @@ -16994,62 +17002,84 @@ typedef struct { static int ScandirIterator_is_closed(ScandirIterator *iterator) { - return iterator->handle == INVALID_HANDLE_VALUE; + return _Py_atomic_load_uint8(&iterator->closed); } static void ScandirIterator_closedir(ScandirIterator *iterator) { - HANDLE handle = iterator->handle; + HANDLE handle = INVALID_HANDLE_VALUE; - if (handle == INVALID_HANDLE_VALUE) - return; + _Py_atomic_store_uint8(&iterator->closed, 1); + if (PyMutex_LockFast(&iterator->read_mutex)) { + // no reads in progress, we can close the handle + handle = iterator->handle; + iterator->handle = INVALID_HANDLE_VALUE; + PyMutex_Unlock(&iterator->read_mutex); + } - iterator->handle = INVALID_HANDLE_VALUE; - Py_BEGIN_ALLOW_THREADS - FindClose(handle); - Py_END_ALLOW_THREADS + 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; + PyMutex_Lock(&iterator->read_mutex); /* 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 && + !_Py_atomic_load_uint8_relaxed(&iterator->closed)) + { 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 */ } + 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)); + 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); @@ -17061,27 +17091,32 @@ ScandirIterator_iternext(PyObject *op) static int ScandirIterator_is_closed(ScandirIterator *iterator) { - return !iterator->dirp; + return _Py_atomic_load_uint8(&iterator->closed); } static void ScandirIterator_closedir(ScandirIterator *iterator) { - DIR *dirp = iterator->dirp; + DIR *dirp = NULL; - if (!dirp) - return; + _Py_atomic_store_uint8(&iterator->closed, 1); + if (PyMutex_LockFast(&iterator->read_mutex)) { + // no reads in progress, we can close dirp + dirp = iterator->dirp; + iterator->dirp = NULL; + PyMutex_Unlock(&iterator->read_mutex); + } - iterator->dirp = NULL; - Py_BEGIN_ALLOW_THREADS + 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 * @@ -17089,24 +17124,32 @@ 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 namebuf[256]; + char *name = namebuf; + ino_t d_ino = 0; +#ifdef HAVE_DIRENT_D_TYPE + unsigned char d_type = 0; +#endif + PyMutex_Lock(&iterator->read_mutex); /* Happens if the iterator is iterated twice, or closed explicitly */ - if (!iterator->dirp) - return NULL; - - while (1) { - errno = 0; + while (iterator->dirp != NULL && + !_Py_atomic_load_uint8_relaxed(&iterator->closed)) + { 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; } @@ -17115,21 +17158,54 @@ 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 + 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'; + 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 */ } + 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)); + PyObject *entry = DirEntry_from_posix_info(module, + &iterator->path, name, + name_len, d_ino +#ifdef HAVE_DIRENT_D_TYPE + , d_type +#endif + ); + if (name != namebuf) { + 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); @@ -17167,9 +17243,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)) { @@ -17267,6 +17345,8 @@ os_scandir_impl(PyObject *module, path_t *path) if (!iterator) return NULL; + iterator->read_mutex = (PyMutex){0}; + iterator->closed = 1; #ifdef MS_WINDOWS iterator->handle = INVALID_HANDLE_VALUE; #else @@ -17339,6 +17419,7 @@ os_scandir_impl(PyObject *module, path_t *path) } #endif + iterator->closed = 0; return (PyObject *)iterator; error: _______________________________________________ Python-checkins mailing list -- [email protected] To unsubscribe send an email to [email protected] https://mail.python.org/mailman3//lists/python-checkins.python.org Member address: [email protected]
