Skip to content

Commit 4ff1243

Browse files
deadlovelllnascheme
andcommitted
gh-152754: Fix crash when an os.scandir iterator is shared between threads
Co-authored-by: Neil Schemenauer <nas-github@arctrix.com>
1 parent 6d38668 commit 4ff1243

3 files changed

Lines changed: 197 additions & 56 deletions

File tree

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import os
2+
import shutil
3+
import tempfile
4+
import threading
5+
import unittest
6+
7+
from test import support
8+
from test.support import threading_helper
9+
10+
11+
if support.check_sanitizer(thread=True):
12+
NUMITEMS = 200
13+
N_NEXT = 2
14+
N_CLOSE = 2
15+
REPEAT = 10
16+
else:
17+
NUMITEMS = 1000
18+
N_NEXT = 6
19+
N_CLOSE = 3
20+
REPEAT = 20
21+
22+
23+
@threading_helper.requires_working_threading()
24+
class ScandirThreadingTest(unittest.TestCase):
25+
def setUp(self):
26+
self.dir = tempfile.mkdtemp()
27+
self.addCleanup(shutil.rmtree, self.dir, ignore_errors=True)
28+
self.names = set()
29+
for i in range(NUMITEMS):
30+
name = f"f{i}"
31+
with open(os.path.join(self.dir, name), "w"):
32+
pass
33+
self.names.add(name)
34+
35+
def run_threads(self, funcs):
36+
threading_helper.run_concurrently(funcs)
37+
38+
def test_close_racing_next(self):
39+
# gh-152754: one thread's next() racing another's close() must not crash.
40+
def nexter():
41+
for _ in self.it:
42+
pass
43+
44+
def closer():
45+
self.it.close()
46+
47+
for _ in range(REPEAT):
48+
self.it = os.scandir(self.dir)
49+
try:
50+
self.run_threads([nexter] * N_NEXT + [closer] * N_CLOSE)
51+
finally:
52+
self.it.close()
53+
54+
def test_shared_next(self):
55+
# gh-152754: threads sharing one iterator must not crash or lose entries.
56+
self.it = os.scandir(self.dir)
57+
results = []
58+
results_lock = threading.Lock()
59+
60+
def worker():
61+
local = []
62+
for entry in self.it:
63+
local.append(entry.name)
64+
with results_lock:
65+
results.extend(local)
66+
67+
try:
68+
self.run_threads([worker] * (N_NEXT + N_CLOSE))
69+
finally:
70+
self.it.close()
71+
72+
self.assertEqual(sorted(results), sorted(self.names))
73+
74+
75+
if __name__ == "__main__":
76+
unittest.main()
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix a crash when the same :func:`os.scandir` iterator is used concurrently
2+
from multiple threads on the :term:`free-threaded <free threading>` build.

Modules/posixmodule.c

Lines changed: 119 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -16821,6 +16821,11 @@ DirEntry_from_posix_info(PyObject *module, path_t *path, const char *name,
1682116821

1682216822
typedef struct {
1682316823
PyObject_HEAD
16824+
#ifdef Py_GIL_DISABLED
16825+
// Protects scandir iterator state when a os.scandir() iterator is used
16826+
// from multiple threads.
16827+
PyMutex mutex;
16828+
#endif
1682416829
path_t path;
1682516830
#ifdef MS_WINDOWS
1682616831
HANDLE handle;
@@ -16836,67 +16841,89 @@ typedef struct {
1683616841

1683716842
#define ScandirIterator_CAST(op) ((ScandirIterator *)(op))
1683816843

16844+
#ifdef Py_GIL_DISABLED
16845+
# define SCANDIR_ITERATOR_LOCK(iterator) PyMutex_Lock(&(iterator)->mutex)
16846+
# define SCANDIR_ITERATOR_UNLOCK(iterator) PyMutex_Unlock(&(iterator)->mutex)
16847+
#else
16848+
# define SCANDIR_ITERATOR_LOCK(iterator) ((void)0)
16849+
# define SCANDIR_ITERATOR_UNLOCK(iterator) ((void)0)
16850+
#endif
16851+
1683916852
#ifdef MS_WINDOWS
1684016853

1684116854
static int
1684216855
ScandirIterator_is_closed(ScandirIterator *iterator)
1684316856
{
16844-
return iterator->handle == INVALID_HANDLE_VALUE;
16857+
SCANDIR_ITERATOR_LOCK(iterator);
16858+
int closed = iterator->handle == INVALID_HANDLE_VALUE;
16859+
SCANDIR_ITERATOR_UNLOCK(iterator);
16860+
return closed;
1684516861
}
1684616862

1684716863
static void
1684816864
ScandirIterator_closedir(ScandirIterator *iterator)
1684916865
{
16866+
SCANDIR_ITERATOR_LOCK(iterator);
1685016867
HANDLE handle = iterator->handle;
16851-
16852-
if (handle == INVALID_HANDLE_VALUE)
16853-
return;
16854-
1685516868
iterator->handle = INVALID_HANDLE_VALUE;
16856-
Py_BEGIN_ALLOW_THREADS
16857-
FindClose(handle);
16858-
Py_END_ALLOW_THREADS
16869+
SCANDIR_ITERATOR_UNLOCK(iterator);
16870+
16871+
if (handle != INVALID_HANDLE_VALUE) {
16872+
Py_BEGIN_ALLOW_THREADS
16873+
FindClose(handle);
16874+
Py_END_ALLOW_THREADS
16875+
}
1685916876
}
1686016877

1686116878
static PyObject *
1686216879
ScandirIterator_iternext(PyObject *op)
1686316880
{
1686416881
ScandirIterator *iterator = ScandirIterator_CAST(op);
16865-
WIN32_FIND_DATAW *file_data = &iterator->file_data;
16882+
WIN32_FIND_DATAW file_data;
1686616883
BOOL success;
16867-
PyObject *entry;
16884+
DWORD error = ERROR_SUCCESS;
16885+
int found = 0;
1686816886

16887+
SCANDIR_ITERATOR_LOCK(iterator);
1686916888
/* Happens if the iterator is iterated twice, or closed explicitly */
16870-
if (iterator->handle == INVALID_HANDLE_VALUE)
16871-
return NULL;
16872-
16873-
while (1) {
16889+
while (iterator->handle != INVALID_HANDLE_VALUE) {
1687416890
if (!iterator->first_time) {
1687516891
Py_BEGIN_ALLOW_THREADS
16876-
success = FindNextFileW(iterator->handle, file_data);
16892+
success = FindNextFileW(iterator->handle, &iterator->file_data);
16893+
if (!success) {
16894+
error = GetLastError();
16895+
}
1687716896
Py_END_ALLOW_THREADS
1687816897
if (!success) {
16879-
/* Error or no more files */
16880-
if (GetLastError() != ERROR_NO_MORE_FILES)
16881-
path_error(&iterator->path);
1688216898
break;
1688316899
}
1688416900
}
1688516901
iterator->first_time = 0;
1688616902

1688716903
/* Skip over . and .. */
16888-
if (wcscmp(file_data->cFileName, L".") != 0 &&
16889-
wcscmp(file_data->cFileName, L"..") != 0)
16904+
if (wcscmp(iterator->file_data.cFileName, L".") != 0 &&
16905+
wcscmp(iterator->file_data.cFileName, L"..") != 0)
1689016906
{
16891-
PyObject *module = PyType_GetModule(Py_TYPE(iterator));
16892-
entry = DirEntry_from_find_data(module, &iterator->path, file_data);
16893-
if (!entry)
16894-
break;
16895-
return entry;
16907+
file_data = iterator->file_data;
16908+
found = 1;
16909+
break;
1689616910
}
1689716911

1689816912
/* Loop till we get a non-dot directory or finish iterating */
1689916913
}
16914+
SCANDIR_ITERATOR_UNLOCK(iterator);
16915+
16916+
if (found) {
16917+
PyObject *module = PyType_GetModule(Py_TYPE(iterator));
16918+
PyObject *entry = DirEntry_from_find_data(module, &iterator->path, &file_data);
16919+
if (entry != NULL) {
16920+
return entry;
16921+
}
16922+
}
16923+
else if (error != ERROR_SUCCESS && error != ERROR_NO_MORE_FILES) {
16924+
SetLastError(error);
16925+
path_error(&iterator->path);
16926+
}
1690016927

1690116928
/* Error or no more files */
1690216929
ScandirIterator_closedir(iterator);
@@ -16908,52 +16935,60 @@ ScandirIterator_iternext(PyObject *op)
1690816935
static int
1690916936
ScandirIterator_is_closed(ScandirIterator *iterator)
1691016937
{
16911-
return !iterator->dirp;
16938+
SCANDIR_ITERATOR_LOCK(iterator);
16939+
int closed = iterator->dirp == NULL;
16940+
SCANDIR_ITERATOR_UNLOCK(iterator);
16941+
return closed;
1691216942
}
1691316943

1691416944
static void
1691516945
ScandirIterator_closedir(ScandirIterator *iterator)
1691616946
{
16947+
SCANDIR_ITERATOR_LOCK(iterator);
1691716948
DIR *dirp = iterator->dirp;
16918-
16919-
if (!dirp)
16920-
return;
16921-
1692216949
iterator->dirp = NULL;
16923-
Py_BEGIN_ALLOW_THREADS
16950+
SCANDIR_ITERATOR_UNLOCK(iterator);
16951+
16952+
if (dirp != NULL) {
16953+
Py_BEGIN_ALLOW_THREADS
1692416954
#ifdef HAVE_FDOPENDIR
16925-
if (iterator->path.is_fd) {
16926-
rewinddir(dirp);
16927-
}
16955+
if (iterator->path.is_fd) {
16956+
rewinddir(dirp);
16957+
}
1692816958
#endif
16929-
closedir(dirp);
16930-
Py_END_ALLOW_THREADS
16931-
return;
16959+
closedir(dirp);
16960+
Py_END_ALLOW_THREADS
16961+
}
1693216962
}
1693316963

1693416964
static PyObject *
1693516965
ScandirIterator_iternext(PyObject *op)
1693616966
{
1693716967
ScandirIterator *iterator = ScandirIterator_CAST(op);
1693816968
struct dirent *direntp;
16939-
Py_ssize_t name_len;
16969+
Py_ssize_t name_len = 0;
1694016970
int is_dot;
16941-
PyObject *entry;
16971+
int found = 0;
16972+
int error = 0;
16973+
int no_memory = 0;
16974+
char *name = NULL;
16975+
ino_t d_ino = 0;
16976+
#ifdef HAVE_DIRENT_D_TYPE
16977+
unsigned char d_type = 0;
16978+
#endif
1694216979

16980+
SCANDIR_ITERATOR_LOCK(iterator);
1694316981
/* Happens if the iterator is iterated twice, or closed explicitly */
16944-
if (!iterator->dirp)
16945-
return NULL;
16946-
16947-
while (1) {
16948-
errno = 0;
16982+
while (iterator->dirp != NULL) {
1694916983
Py_BEGIN_ALLOW_THREADS
16984+
errno = 0;
1695016985
direntp = readdir(iterator->dirp);
16986+
if (direntp == NULL) {
16987+
error = errno;
16988+
}
1695116989
Py_END_ALLOW_THREADS
1695216990

1695316991
if (!direntp) {
16954-
/* Error or no more files */
16955-
if (errno != 0)
16956-
path_error(&iterator->path);
1695716992
break;
1695816993
}
1695916994

@@ -16962,21 +16997,46 @@ ScandirIterator_iternext(PyObject *op)
1696216997
is_dot = direntp->d_name[0] == '.' &&
1696316998
(name_len == 1 || (direntp->d_name[1] == '.' && name_len == 2));
1696416999
if (!is_dot) {
16965-
PyObject *module = PyType_GetModule(Py_TYPE(iterator));
16966-
entry = DirEntry_from_posix_info(module,
16967-
&iterator->path, direntp->d_name,
16968-
name_len, direntp->d_ino
17000+
name = PyMem_RawMalloc(name_len + 1);
17001+
if (name == NULL) {
17002+
no_memory = 1;
17003+
break;
17004+
}
17005+
memcpy(name, direntp->d_name, name_len);
17006+
name[name_len] = '\0';
17007+
d_ino = direntp->d_ino;
1696917008
#ifdef HAVE_DIRENT_D_TYPE
16970-
, direntp->d_type
17009+
d_type = direntp->d_type;
1697117010
#endif
16972-
);
16973-
if (!entry)
16974-
break;
16975-
return entry;
17011+
found = 1;
17012+
break;
1697617013
}
1697717014

1697817015
/* Loop till we get a non-dot directory or finish iterating */
1697917016
}
17017+
SCANDIR_ITERATOR_UNLOCK(iterator);
17018+
17019+
if (found) {
17020+
PyObject *module = PyType_GetModule(Py_TYPE(iterator));
17021+
PyObject *entry = DirEntry_from_posix_info(module,
17022+
&iterator->path, name,
17023+
name_len, d_ino
17024+
#ifdef HAVE_DIRENT_D_TYPE
17025+
, d_type
17026+
#endif
17027+
);
17028+
PyMem_RawFree(name);
17029+
if (entry != NULL) {
17030+
return entry;
17031+
}
17032+
}
17033+
else if (no_memory) {
17034+
PyErr_NoMemory();
17035+
}
17036+
else if (error != 0) {
17037+
errno = error;
17038+
path_error(&iterator->path);
17039+
}
1698017040

1698117041
/* Error or no more files */
1698217042
ScandirIterator_closedir(iterator);
@@ -17114,6 +17174,9 @@ os_scandir_impl(PyObject *module, path_t *path)
1711417174
if (!iterator)
1711517175
return NULL;
1711617176

17177+
#ifdef Py_GIL_DISABLED
17178+
iterator->mutex = (PyMutex){0};
17179+
#endif
1711717180
#ifdef MS_WINDOWS
1711817181
iterator->handle = INVALID_HANDLE_VALUE;
1711917182
#else

0 commit comments

Comments
 (0)