Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 87 additions & 0 deletions Lib/test/test_free_threading/test_os.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
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):
barrier = threading.Barrier(len(funcs))
threads = [threading.Thread(target=f, args=(barrier,)) for f in funcs]
for t in threads:
t.start()
for t in threads:
t.join()

def test_close_racing_next(self):
# gh-152754: one thread's next() racing another's close() must not crash.
def nexter(barrier):
barrier.wait()
try:
for _ in self.it:
pass
except Exception:
pass

def closer(barrier):
barrier.wait()
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(barrier):
local = []
barrier.wait()
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()
Original file line number Diff line number Diff line change
@@ -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 <free threading>` build.
76 changes: 36 additions & 40 deletions Modules/posixmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -16847,15 +16847,14 @@ ScandirIterator_is_closed(ScandirIterator *iterator)
static void
ScandirIterator_closedir(ScandirIterator *iterator)
{
// gh-152754: close under the critical section so it can't race iternext.
Py_BEGIN_CRITICAL_SECTION(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
if (handle != INVALID_HANDLE_VALUE) {
iterator->handle = INVALID_HANDLE_VALUE;
FindClose(handle);
}
Py_END_CRITICAL_SECTION();
}

static PyObject *
Expand All @@ -16864,17 +16863,14 @@ ScandirIterator_iternext(PyObject *op)
ScandirIterator *iterator = ScandirIterator_CAST(op);
WIN32_FIND_DATAW *file_data = &iterator->file_data;
BOOL success;
PyObject *entry;
PyObject *entry = NULL;

// gh-152754: iterate under the critical section so close can't invalidate handle mid-use.
Py_BEGIN_CRITICAL_SECTION(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);
Py_END_ALLOW_THREADS
if (!success) {
/* Error or no more files */
if (GetLastError() != ERROR_NO_MORE_FILES)
Expand All @@ -16890,14 +16886,17 @@ ScandirIterator_iternext(PyObject *op)
{
PyObject *module = PyType_GetModule(Py_TYPE(iterator));
entry = DirEntry_from_find_data(module, &iterator->path, file_data);
if (!entry)
break;
return entry;
break;
}

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

Py_END_CRITICAL_SECTION();

if (entry != NULL)
return entry;

/* Error or no more files */
ScandirIterator_closedir(iterator);
return NULL;
Expand All @@ -16914,21 +16913,19 @@ ScandirIterator_is_closed(ScandirIterator *iterator)
static void
ScandirIterator_closedir(ScandirIterator *iterator)
{
// gh-152754: close under the critical section so it can't race iternext.
Py_BEGIN_CRITICAL_SECTION(iterator);
DIR *dirp = iterator->dirp;

if (!dirp)
return;

iterator->dirp = NULL;
Py_BEGIN_ALLOW_THREADS
if (dirp != NULL) {
iterator->dirp = NULL;
#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_CRITICAL_SECTION();
}

static PyObject *
Expand All @@ -16938,17 +16935,14 @@ ScandirIterator_iternext(PyObject *op)
struct dirent *direntp;
Py_ssize_t name_len;
int is_dot;
PyObject *entry;
PyObject *entry = NULL;

// gh-152754: iterate under the critical section so close can't free dirp mid-use.
Py_BEGIN_CRITICAL_SECTION(iterator);
/* Happens if the iterator is iterated twice, or closed explicitly */
if (!iterator->dirp)
return NULL;

while (1) {
while (iterator->dirp != NULL) {
errno = 0;
Py_BEGIN_ALLOW_THREADS
direntp = readdir(iterator->dirp);
Py_END_ALLOW_THREADS

if (!direntp) {
/* Error or no more files */
Expand All @@ -16970,13 +16964,15 @@ ScandirIterator_iternext(PyObject *op)
, direntp->d_type
#endif
);
if (!entry)
break;
return entry;
break;
}

/* Loop till we get a non-dot directory or finish iterating */
}
Py_END_CRITICAL_SECTION();

if (entry != NULL)
return entry;

/* Error or no more files */
ScandirIterator_closedir(iterator);
Expand Down
Loading