Skip to content

Commit 3a7333f

Browse files
committed
gh-152754: Fix crash when an os.scandir iterator is shared between threads
1 parent c843cb7 commit 3a7333f

3 files changed

Lines changed: 125 additions & 40 deletions

File tree

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
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+
barrier = threading.Barrier(len(funcs))
37+
threads = [threading.Thread(target=f, args=(barrier,)) for f in funcs]
38+
for t in threads:
39+
t.start()
40+
for t in threads:
41+
t.join()
42+
43+
def test_close_racing_next(self):
44+
# gh-152754: one thread's next() racing another's close() must not crash.
45+
def nexter(barrier):
46+
barrier.wait()
47+
try:
48+
for _ in self.it:
49+
pass
50+
except Exception:
51+
pass
52+
53+
def closer(barrier):
54+
barrier.wait()
55+
self.it.close()
56+
57+
for _ in range(REPEAT):
58+
self.it = os.scandir(self.dir)
59+
try:
60+
self.run_threads([nexter] * N_NEXT + [closer] * N_CLOSE)
61+
finally:
62+
self.it.close()
63+
64+
def test_shared_next(self):
65+
# gh-152754: threads sharing one iterator must not crash or lose entries.
66+
self.it = os.scandir(self.dir)
67+
results = []
68+
results_lock = threading.Lock()
69+
70+
def worker(barrier):
71+
local = []
72+
barrier.wait()
73+
for entry in self.it:
74+
local.append(entry.name)
75+
with results_lock:
76+
results.extend(local)
77+
78+
try:
79+
self.run_threads([worker] * (N_NEXT + N_CLOSE))
80+
finally:
81+
self.it.close()
82+
83+
self.assertEqual(sorted(results), sorted(self.names))
84+
85+
86+
if __name__ == "__main__":
87+
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: 36 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -16847,15 +16847,14 @@ ScandirIterator_is_closed(ScandirIterator *iterator)
1684716847
static void
1684816848
ScandirIterator_closedir(ScandirIterator *iterator)
1684916849
{
16850+
// gh-152754: close under the critical section so it can't race iternext.
16851+
Py_BEGIN_CRITICAL_SECTION(iterator);
1685016852
HANDLE handle = iterator->handle;
16851-
16852-
if (handle == INVALID_HANDLE_VALUE)
16853-
return;
16854-
16855-
iterator->handle = INVALID_HANDLE_VALUE;
16856-
Py_BEGIN_ALLOW_THREADS
16857-
FindClose(handle);
16858-
Py_END_ALLOW_THREADS
16853+
if (handle != INVALID_HANDLE_VALUE) {
16854+
iterator->handle = INVALID_HANDLE_VALUE;
16855+
FindClose(handle);
16856+
}
16857+
Py_END_CRITICAL_SECTION();
1685916858
}
1686016859

1686116860
static PyObject *
@@ -16864,17 +16863,14 @@ ScandirIterator_iternext(PyObject *op)
1686416863
ScandirIterator *iterator = ScandirIterator_CAST(op);
1686516864
WIN32_FIND_DATAW *file_data = &iterator->file_data;
1686616865
BOOL success;
16867-
PyObject *entry;
16866+
PyObject *entry = NULL;
1686816867

16868+
// gh-152754: iterate under the critical section so close can't invalidate handle mid-use.
16869+
Py_BEGIN_CRITICAL_SECTION(iterator);
1686916870
/* Happens if the iterator is iterated twice, or closed explicitly */
16870-
if (iterator->handle == INVALID_HANDLE_VALUE)
16871-
return NULL;
16872-
16873-
while (1) {
16871+
while (iterator->handle != INVALID_HANDLE_VALUE) {
1687416872
if (!iterator->first_time) {
16875-
Py_BEGIN_ALLOW_THREADS
1687616873
success = FindNextFileW(iterator->handle, file_data);
16877-
Py_END_ALLOW_THREADS
1687816874
if (!success) {
1687916875
/* Error or no more files */
1688016876
if (GetLastError() != ERROR_NO_MORE_FILES)
@@ -16890,14 +16886,17 @@ ScandirIterator_iternext(PyObject *op)
1689016886
{
1689116887
PyObject *module = PyType_GetModule(Py_TYPE(iterator));
1689216888
entry = DirEntry_from_find_data(module, &iterator->path, file_data);
16893-
if (!entry)
16894-
break;
16895-
return entry;
16889+
break;
1689616890
}
1689716891

1689816892
/* Loop till we get a non-dot directory or finish iterating */
1689916893
}
1690016894

16895+
Py_END_CRITICAL_SECTION();
16896+
16897+
if (entry != NULL)
16898+
return entry;
16899+
1690116900
/* Error or no more files */
1690216901
ScandirIterator_closedir(iterator);
1690316902
return NULL;
@@ -16914,21 +16913,19 @@ ScandirIterator_is_closed(ScandirIterator *iterator)
1691416913
static void
1691516914
ScandirIterator_closedir(ScandirIterator *iterator)
1691616915
{
16916+
// gh-152754: close under the critical section so it can't race iternext.
16917+
Py_BEGIN_CRITICAL_SECTION(iterator);
1691716918
DIR *dirp = iterator->dirp;
16918-
16919-
if (!dirp)
16920-
return;
16921-
16922-
iterator->dirp = NULL;
16923-
Py_BEGIN_ALLOW_THREADS
16919+
if (dirp != NULL) {
16920+
iterator->dirp = NULL;
1692416921
#ifdef HAVE_FDOPENDIR
16925-
if (iterator->path.is_fd) {
16926-
rewinddir(dirp);
16927-
}
16922+
if (iterator->path.is_fd) {
16923+
rewinddir(dirp);
16924+
}
1692816925
#endif
16929-
closedir(dirp);
16930-
Py_END_ALLOW_THREADS
16931-
return;
16926+
closedir(dirp);
16927+
}
16928+
Py_END_CRITICAL_SECTION();
1693216929
}
1693316930

1693416931
static PyObject *
@@ -16938,17 +16935,14 @@ ScandirIterator_iternext(PyObject *op)
1693816935
struct dirent *direntp;
1693916936
Py_ssize_t name_len;
1694016937
int is_dot;
16941-
PyObject *entry;
16938+
PyObject *entry = NULL;
1694216939

16940+
// gh-152754: iterate under the critical section so close can't free dirp mid-use.
16941+
Py_BEGIN_CRITICAL_SECTION(iterator);
1694316942
/* Happens if the iterator is iterated twice, or closed explicitly */
16944-
if (!iterator->dirp)
16945-
return NULL;
16946-
16947-
while (1) {
16943+
while (iterator->dirp != NULL) {
1694816944
errno = 0;
16949-
Py_BEGIN_ALLOW_THREADS
1695016945
direntp = readdir(iterator->dirp);
16951-
Py_END_ALLOW_THREADS
1695216946

1695316947
if (!direntp) {
1695416948
/* Error or no more files */
@@ -16970,13 +16964,15 @@ ScandirIterator_iternext(PyObject *op)
1697016964
, direntp->d_type
1697116965
#endif
1697216966
);
16973-
if (!entry)
16974-
break;
16975-
return entry;
16967+
break;
1697616968
}
1697716969

1697816970
/* Loop till we get a non-dot directory or finish iterating */
1697916971
}
16972+
Py_END_CRITICAL_SECTION();
16973+
16974+
if (entry != NULL)
16975+
return entry;
1698016976

1698116977
/* Error or no more files */
1698216978
ScandirIterator_closedir(iterator);

0 commit comments

Comments
 (0)