From 5a74e0ea5a2ca14d742b676553b917be1dcf5754 Mon Sep 17 00:00:00 2001 From: alperyoney Date: Wed, 11 Jun 2025 22:08:12 -0700 Subject: [PATCH 1/9] gh-116738: Make grp module thread-safe --- Lib/test/test_free_threading/test_ft.py | 52 +++++++++++++++++++ Lib/test/test_free_threading/test_grp.py | 36 +++++++++++++ Lib/test/test_free_threading/test_heapq.py | 46 +++++----------- ...-06-12-00-03-34.gh-issue-116738.iBBAdo.rst | 1 + Modules/grpmodule.c | 33 ++++++++++-- 5 files changed, 132 insertions(+), 36 deletions(-) create mode 100644 Lib/test/test_free_threading/test_ft.py create mode 100644 Lib/test/test_free_threading/test_grp.py create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-06-12-00-03-34.gh-issue-116738.iBBAdo.rst diff --git a/Lib/test/test_free_threading/test_ft.py b/Lib/test/test_free_threading/test_ft.py new file mode 100644 index 00000000000000..95feb6b009817a --- /dev/null +++ b/Lib/test/test_free_threading/test_ft.py @@ -0,0 +1,52 @@ +import unittest + +from threading import Thread, Barrier +from test.support import threading_helper + + +def run_concurrently(worker_func, args, nthreads): + """ + Run the worker function concurrently in multiple threads. + """ + barrier = Barrier(nthreads) + + def wrapper_func(*args): + # Wait for all threads to reach this point before proceeding. + barrier.wait() + worker_func(*args) + + with threading_helper.catch_threading_exception() as cm: + workers = ( + Thread(target=wrapper_func, args=args) for _ in range(nthreads) + ) + with threading_helper.start_threads(workers): + pass + + # If a worker thread raises an exception, re-raise it. + if cm.exc_value is not None: + raise cm.exc_value + + +@threading_helper.requires_working_threading() +class TestFTUtils(unittest.TestCase): + def test_run_concurrently(self): + lst = [] + + def worker(lst): + lst.append(42) + + nthreads = 10 + run_concurrently(worker, (lst,), nthreads) + self.assertEqual(lst, [42] * nthreads) + + def test_run_concurrently_raise(self): + def worker(): + raise RuntimeError("Error") + + nthreads = 3 + with self.assertRaises(RuntimeError): + run_concurrently(worker, (), nthreads) + + +if __name__ == "__main__": + unittest.main() diff --git a/Lib/test/test_free_threading/test_grp.py b/Lib/test/test_free_threading/test_grp.py new file mode 100644 index 00000000000000..af4e5b6bb73dfd --- /dev/null +++ b/Lib/test/test_free_threading/test_grp.py @@ -0,0 +1,36 @@ +import unittest + +from test.support import import_helper, threading_helper +from test.test_free_threading.test_ft import run_concurrently + +grp = import_helper.import_module("grp") + +from test import test_grp + + +NTHREADS = 10 + + +@threading_helper.requires_working_threading() +class TestGrp(unittest.TestCase): + def setUp(self): + self.test_grp = test_grp.GroupDatabaseTestCase() + + def test_racing_test_values(self): + # test_grp.test_values() calls grp.getgrall() and checks the entries + run_concurrently( + worker_func=self.test_grp.test_values, args=(), nthreads=NTHREADS + ) + + def test_racing_test_values_extended(self): + # test_grp.test_values_extended() calls grp.getgrall(), grp.getgrgid(), + # grp.getgrnam() and checks the entries + run_concurrently( + worker_func=self.test_grp.test_values_extended, + args=(), + nthreads=NTHREADS, + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/Lib/test/test_free_threading/test_heapq.py b/Lib/test/test_free_threading/test_heapq.py index ee7adfb2b78d83..d959fb78e315b0 100644 --- a/Lib/test/test_free_threading/test_heapq.py +++ b/Lib/test/test_free_threading/test_heapq.py @@ -3,10 +3,11 @@ import heapq from enum import Enum -from threading import Thread, Barrier, Lock +from threading import Barrier, Lock from random import shuffle, randint from test.support import threading_helper +from test.test_free_threading.test_ft import run_concurrently from test import test_heapq @@ -28,7 +29,7 @@ def test_racing_heapify(self): heap = list(range(OBJECT_COUNT)) shuffle(heap) - self.run_concurrently( + run_concurrently( worker_func=heapq.heapify, args=(heap,), nthreads=NTHREADS ) self.test_heapq.check_invariant(heap) @@ -40,7 +41,7 @@ def heappush_func(heap): for item in reversed(range(OBJECT_COUNT)): heapq.heappush(heap, item) - self.run_concurrently( + run_concurrently( worker_func=heappush_func, args=(heap,), nthreads=NTHREADS ) self.test_heapq.check_invariant(heap) @@ -61,7 +62,7 @@ def heappop_func(heap, pop_count): # Each local list should be sorted self.assertTrue(self.is_sorted_ascending(local_list)) - self.run_concurrently( + run_concurrently( worker_func=heappop_func, args=(heap, per_thread_pop_count), nthreads=NTHREADS, @@ -77,7 +78,7 @@ def heappushpop_func(heap, pushpop_items): popped_item = heapq.heappushpop(heap, item) self.assertTrue(popped_item <= item) - self.run_concurrently( + run_concurrently( worker_func=heappushpop_func, args=(heap, pushpop_items), nthreads=NTHREADS, @@ -93,7 +94,7 @@ def heapreplace_func(heap, replace_items): for item in replace_items: heapq.heapreplace(heap, item) - self.run_concurrently( + run_concurrently( worker_func=heapreplace_func, args=(heap, replace_items), nthreads=NTHREADS, @@ -105,7 +106,7 @@ def test_racing_heapify_max(self): max_heap = list(range(OBJECT_COUNT)) shuffle(max_heap) - self.run_concurrently( + run_concurrently( worker_func=heapq.heapify_max, args=(max_heap,), nthreads=NTHREADS ) self.test_heapq.check_max_invariant(max_heap) @@ -117,7 +118,7 @@ def heappush_max_func(max_heap): for item in range(OBJECT_COUNT): heapq.heappush_max(max_heap, item) - self.run_concurrently( + run_concurrently( worker_func=heappush_max_func, args=(max_heap,), nthreads=NTHREADS ) self.test_heapq.check_max_invariant(max_heap) @@ -138,7 +139,7 @@ def heappop_max_func(max_heap, pop_count): # Each local list should be sorted self.assertTrue(self.is_sorted_descending(local_list)) - self.run_concurrently( + run_concurrently( worker_func=heappop_max_func, args=(max_heap, per_thread_pop_count), nthreads=NTHREADS, @@ -154,7 +155,7 @@ def heappushpop_max_func(max_heap, pushpop_items): popped_item = heapq.heappushpop_max(max_heap, item) self.assertTrue(popped_item >= item) - self.run_concurrently( + run_concurrently( worker_func=heappushpop_max_func, args=(max_heap, pushpop_items), nthreads=NTHREADS, @@ -170,7 +171,7 @@ def heapreplace_max_func(max_heap, replace_items): for item in replace_items: heapq.heapreplace_max(max_heap, item) - self.run_concurrently( + run_concurrently( worker_func=heapreplace_max_func, args=(max_heap, replace_items), nthreads=NTHREADS, @@ -203,7 +204,7 @@ def worker(): except IndexError: pass - self.run_concurrently(worker, (), n_threads * 2) + run_concurrently(worker, (), n_threads * 2) @staticmethod def is_sorted_ascending(lst): @@ -241,27 +242,6 @@ def create_random_list(a, b, size): """ return [randint(-a, b) for _ in range(size)] - def run_concurrently(self, worker_func, args, nthreads): - """ - Run the worker function concurrently in multiple threads. - """ - barrier = Barrier(nthreads) - - def wrapper_func(*args): - # Wait for all threads to reach this point before proceeding. - barrier.wait() - worker_func(*args) - - with threading_helper.catch_threading_exception() as cm: - workers = ( - Thread(target=wrapper_func, args=args) for _ in range(nthreads) - ) - with threading_helper.start_threads(workers): - pass - - # Worker threads should not raise any exceptions - self.assertIsNone(cm.exc_value) - if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-06-12-00-03-34.gh-issue-116738.iBBAdo.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-12-00-03-34.gh-issue-116738.iBBAdo.rst new file mode 100644 index 00000000000000..1f3c53c3464ace --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-12-00-03-34.gh-issue-116738.iBBAdo.rst @@ -0,0 +1 @@ +Make methods in :mod:`grp` thread-safe on the :term:`free threaded ` build. diff --git a/Modules/grpmodule.c b/Modules/grpmodule.c index 29da9936b65504..35350f9de53fa5 100644 --- a/Modules/grpmodule.c +++ b/Modules/grpmodule.c @@ -132,7 +132,7 @@ grp_getgrgid_impl(PyObject *module, PyObject *id) if (!_Py_Gid_Converter(id, &gid)) { return NULL; } -#ifdef HAVE_GETGRGID_R +#if defined(HAVE_GETGRGID_R) int status; Py_ssize_t bufsize; /* Note: 'grp' will be used via pointer 'p' on getgrgid_r success. */ @@ -167,6 +167,17 @@ grp_getgrgid_impl(PyObject *module, PyObject *id) } Py_END_ALLOW_THREADS +#elif defined(Py_GIL_DISABLED) + static PyMutex getgrgid_mutex = {0}; + PyMutex_Lock(&getgrgid_mutex); + // The getgrgid() function need not be thread-safe. + // https://pubs.opengroup.org/onlinepubs/9699919799/functions/getgrgid.html + p = getgrgid(gid); + if (p == NULL) { + // Unlock the mutex on error. The following error handling block will + // handle the rest. + PyMutex_Unlock(&getgrgid_mutex); + } #else p = getgrgid(gid); #endif @@ -183,8 +194,10 @@ grp_getgrgid_impl(PyObject *module, PyObject *id) return NULL; } retval = mkgrent(module, p); -#ifdef HAVE_GETGRGID_R +#if defined(HAVE_GETGRGID_R) PyMem_RawFree(buf); +#elif defined(Py_GIL_DISABLED) + PyMutex_Unlock(&getgrgid_mutex); #endif return retval; } @@ -213,7 +226,7 @@ grp_getgrnam_impl(PyObject *module, PyObject *name) /* check for embedded null bytes */ if (PyBytes_AsStringAndSize(bytes, &name_chars, NULL) == -1) goto out; -#ifdef HAVE_GETGRNAM_R +#if defined(HAVE_GETGRNAM_R) int status; Py_ssize_t bufsize; /* Note: 'grp' will be used via pointer 'p' on getgrnam_r success. */ @@ -248,6 +261,17 @@ grp_getgrnam_impl(PyObject *module, PyObject *name) } Py_END_ALLOW_THREADS +#elif defined(Py_GIL_DISABLED) + static PyMutex getgrnam_mutex = {0}; + PyMutex_Lock(&getgrnam_mutex); + // The getgrnam() function need not be thread-safe. + // https://pubs.opengroup.org/onlinepubs/9699919799/functions/getgrnam.html + p = getgrnam(name_chars); + if (p == NULL) { + // Unlock the mutex on error. The following error handling block will + // handle the rest. + PyMutex_Unlock(&getgrnam_mutex); + } #else p = getgrnam(name_chars); #endif @@ -261,6 +285,9 @@ grp_getgrnam_impl(PyObject *module, PyObject *name) goto out; } retval = mkgrent(module, p); +#if !defined(HAVE_GETGRNAM_R) && defined(Py_GIL_DISABLED) + PyMutex_Unlock(&getgrnam_mutex); +#endif out: PyMem_RawFree(buf); Py_DECREF(bytes); From 63360a0581fa0b7cceed46a0542b34722880bd1c Mon Sep 17 00:00:00 2001 From: alperyoney Date: Mon, 16 Jun 2025 14:39:38 -0700 Subject: [PATCH 2/9] gh-116738: Protect non thread-safe functions in default build --- Modules/grpmodule.c | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/Modules/grpmodule.c b/Modules/grpmodule.c index 35350f9de53fa5..b1916ae78885fe 100644 --- a/Modules/grpmodule.c +++ b/Modules/grpmodule.c @@ -132,7 +132,7 @@ grp_getgrgid_impl(PyObject *module, PyObject *id) if (!_Py_Gid_Converter(id, &gid)) { return NULL; } -#if defined(HAVE_GETGRGID_R) +#ifdef HAVE_GETGRGID_R int status; Py_ssize_t bufsize; /* Note: 'grp' will be used via pointer 'p' on getgrgid_r success. */ @@ -167,21 +167,17 @@ grp_getgrgid_impl(PyObject *module, PyObject *id) } Py_END_ALLOW_THREADS -#elif defined(Py_GIL_DISABLED) +#else static PyMutex getgrgid_mutex = {0}; PyMutex_Lock(&getgrgid_mutex); // The getgrgid() function need not be thread-safe. // https://pubs.opengroup.org/onlinepubs/9699919799/functions/getgrgid.html p = getgrgid(gid); +#endif if (p == NULL) { - // Unlock the mutex on error. The following error handling block will - // handle the rest. +#ifndef HAVE_GETGRGID_R PyMutex_Unlock(&getgrgid_mutex); - } -#else - p = getgrgid(gid); #endif - if (p == NULL) { PyMem_RawFree(buf); if (nomem == 1) { return PyErr_NoMemory(); @@ -194,9 +190,9 @@ grp_getgrgid_impl(PyObject *module, PyObject *id) return NULL; } retval = mkgrent(module, p); -#if defined(HAVE_GETGRGID_R) +#ifdef HAVE_GETGRGID_R PyMem_RawFree(buf); -#elif defined(Py_GIL_DISABLED) +#else PyMutex_Unlock(&getgrgid_mutex); #endif return retval; @@ -226,7 +222,7 @@ grp_getgrnam_impl(PyObject *module, PyObject *name) /* check for embedded null bytes */ if (PyBytes_AsStringAndSize(bytes, &name_chars, NULL) == -1) goto out; -#if defined(HAVE_GETGRNAM_R) +#ifdef HAVE_GETGRNAM_R int status; Py_ssize_t bufsize; /* Note: 'grp' will be used via pointer 'p' on getgrnam_r success. */ @@ -261,21 +257,17 @@ grp_getgrnam_impl(PyObject *module, PyObject *name) } Py_END_ALLOW_THREADS -#elif defined(Py_GIL_DISABLED) +#else static PyMutex getgrnam_mutex = {0}; PyMutex_Lock(&getgrnam_mutex); // The getgrnam() function need not be thread-safe. // https://pubs.opengroup.org/onlinepubs/9699919799/functions/getgrnam.html p = getgrnam(name_chars); +#endif if (p == NULL) { - // Unlock the mutex on error. The following error handling block will - // handle the rest. +#ifndef HAVE_GETGRGID_R PyMutex_Unlock(&getgrnam_mutex); - } -#else - p = getgrnam(name_chars); #endif - if (p == NULL) { if (nomem == 1) { PyErr_NoMemory(); } @@ -285,7 +277,7 @@ grp_getgrnam_impl(PyObject *module, PyObject *name) goto out; } retval = mkgrent(module, p); -#if !defined(HAVE_GETGRNAM_R) && defined(Py_GIL_DISABLED) +#ifndef HAVE_GETGRNAM_R PyMutex_Unlock(&getgrnam_mutex); #endif out: From c6861c6618e32b2507da19fbb01fe53cf04fe030 Mon Sep 17 00:00:00 2001 From: alperyoney Date: Mon, 16 Jun 2025 15:27:06 -0700 Subject: [PATCH 3/9] gh-116738: Fix pre-processor block --- Modules/grpmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/grpmodule.c b/Modules/grpmodule.c index b1916ae78885fe..b33f5e472dee98 100644 --- a/Modules/grpmodule.c +++ b/Modules/grpmodule.c @@ -265,7 +265,7 @@ grp_getgrnam_impl(PyObject *module, PyObject *name) p = getgrnam(name_chars); #endif if (p == NULL) { -#ifndef HAVE_GETGRGID_R +#ifndef HAVE_GETGRNAM_R PyMutex_Unlock(&getgrnam_mutex); #endif if (nomem == 1) { From e879eed7a7b037ca1733ca981a1814b44d705c5b Mon Sep 17 00:00:00 2001 From: alperyoney Date: Tue, 17 Jun 2025 12:58:30 -0700 Subject: [PATCH 4/9] gh-116738: Move run_concurrently() to threading_helper --- Lib/test/support/threading_helper.py | 24 ++++++++++ Lib/test/test_free_threading/test_ft.py | 52 ---------------------- Lib/test/test_free_threading/test_grp.py | 2 +- Lib/test/test_free_threading/test_heapq.py | 2 +- 4 files changed, 26 insertions(+), 54 deletions(-) delete mode 100644 Lib/test/test_free_threading/test_ft.py diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index afa25a76f63829..7724f863ca42f0 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -248,3 +248,27 @@ def requires_working_threading(*, module=False): raise unittest.SkipTest(msg) else: return unittest.skipUnless(can_start_thread, msg) + + +def run_concurrently(worker_func, args, nthreads): + """ + Run the worker function concurrently in multiple threads. + """ + barrier = threading.Barrier(nthreads) + + def wrapper_func(*args): + # Wait for all threads to reach this point before proceeding. + barrier.wait() + worker_func(*args) + + with catch_threading_exception() as cm: + workers = ( + threading.Thread(target=wrapper_func, args=args) + for _ in range(nthreads) + ) + with start_threads(workers): + pass + + # If a worker thread raises an exception, re-raise it. + if cm.exc_value is not None: + raise cm.exc_value diff --git a/Lib/test/test_free_threading/test_ft.py b/Lib/test/test_free_threading/test_ft.py deleted file mode 100644 index 95feb6b009817a..00000000000000 --- a/Lib/test/test_free_threading/test_ft.py +++ /dev/null @@ -1,52 +0,0 @@ -import unittest - -from threading import Thread, Barrier -from test.support import threading_helper - - -def run_concurrently(worker_func, args, nthreads): - """ - Run the worker function concurrently in multiple threads. - """ - barrier = Barrier(nthreads) - - def wrapper_func(*args): - # Wait for all threads to reach this point before proceeding. - barrier.wait() - worker_func(*args) - - with threading_helper.catch_threading_exception() as cm: - workers = ( - Thread(target=wrapper_func, args=args) for _ in range(nthreads) - ) - with threading_helper.start_threads(workers): - pass - - # If a worker thread raises an exception, re-raise it. - if cm.exc_value is not None: - raise cm.exc_value - - -@threading_helper.requires_working_threading() -class TestFTUtils(unittest.TestCase): - def test_run_concurrently(self): - lst = [] - - def worker(lst): - lst.append(42) - - nthreads = 10 - run_concurrently(worker, (lst,), nthreads) - self.assertEqual(lst, [42] * nthreads) - - def test_run_concurrently_raise(self): - def worker(): - raise RuntimeError("Error") - - nthreads = 3 - with self.assertRaises(RuntimeError): - run_concurrently(worker, (), nthreads) - - -if __name__ == "__main__": - unittest.main() diff --git a/Lib/test/test_free_threading/test_grp.py b/Lib/test/test_free_threading/test_grp.py index af4e5b6bb73dfd..493475809f04eb 100644 --- a/Lib/test/test_free_threading/test_grp.py +++ b/Lib/test/test_free_threading/test_grp.py @@ -1,7 +1,7 @@ import unittest from test.support import import_helper, threading_helper -from test.test_free_threading.test_ft import run_concurrently +from test.support.threading_helper import run_concurrently grp = import_helper.import_module("grp") diff --git a/Lib/test/test_free_threading/test_heapq.py b/Lib/test/test_free_threading/test_heapq.py index d959fb78e315b0..dd14b4783ed20f 100644 --- a/Lib/test/test_free_threading/test_heapq.py +++ b/Lib/test/test_free_threading/test_heapq.py @@ -7,7 +7,7 @@ from random import shuffle, randint from test.support import threading_helper -from test.test_free_threading.test_ft import run_concurrently +from test.support.threading_helper import run_concurrently from test import test_heapq From 851caf4c754e63ca0ca93adb456eb0f439b80af2 Mon Sep 17 00:00:00 2001 From: alperyoney Date: Thu, 3 Jul 2025 11:11:44 -0700 Subject: [PATCH 5/9] gh-116738: Improve run_concurrently() arguments --- Doc/library/test.rst | 7 +++++++ Lib/test/support/threading_helper.py | 8 ++++---- Lib/test/test_free_threading/test_grp.py | 3 +-- Lib/test/test_free_threading/test_heapq.py | 22 +++++++++++----------- 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/Doc/library/test.rst b/Doc/library/test.rst index 0aae14c15a6104..9fdb21b8badbbf 100644 --- a/Doc/library/test.rst +++ b/Doc/library/test.rst @@ -1384,6 +1384,13 @@ The :mod:`test.support.threading_helper` module provides support for threading t .. versionadded:: 3.8 +.. function:: run_concurrently(worker_func, nthreads, args=(), kwargs={}) + + Run the worker function concurrently in multiple threads. + Re-raises an exception if any thread raises one, after all threads have + finished. + + :mod:`test.support.os_helper` --- Utilities for os tests ======================================================================== diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index 7724f863ca42f0..aa896841229508 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -250,20 +250,20 @@ def requires_working_threading(*, module=False): return unittest.skipUnless(can_start_thread, msg) -def run_concurrently(worker_func, args, nthreads): +def run_concurrently(worker_func, nthreads, args=(), kwargs={}): """ Run the worker function concurrently in multiple threads. """ barrier = threading.Barrier(nthreads) - def wrapper_func(*args): + def wrapper_func(*args, **kwargs): # Wait for all threads to reach this point before proceeding. barrier.wait() - worker_func(*args) + worker_func(*args, **kwargs) with catch_threading_exception() as cm: workers = ( - threading.Thread(target=wrapper_func, args=args) + threading.Thread(target=wrapper_func, args=args, kwargs=kwargs) for _ in range(nthreads) ) with start_threads(workers): diff --git a/Lib/test/test_free_threading/test_grp.py b/Lib/test/test_free_threading/test_grp.py index 493475809f04eb..1a47a9757702be 100644 --- a/Lib/test/test_free_threading/test_grp.py +++ b/Lib/test/test_free_threading/test_grp.py @@ -19,7 +19,7 @@ def setUp(self): def test_racing_test_values(self): # test_grp.test_values() calls grp.getgrall() and checks the entries run_concurrently( - worker_func=self.test_grp.test_values, args=(), nthreads=NTHREADS + worker_func=self.test_grp.test_values, nthreads=NTHREADS ) def test_racing_test_values_extended(self): @@ -27,7 +27,6 @@ def test_racing_test_values_extended(self): # grp.getgrnam() and checks the entries run_concurrently( worker_func=self.test_grp.test_values_extended, - args=(), nthreads=NTHREADS, ) diff --git a/Lib/test/test_free_threading/test_heapq.py b/Lib/test/test_free_threading/test_heapq.py index dd14b4783ed20f..d771333ffcc9e0 100644 --- a/Lib/test/test_free_threading/test_heapq.py +++ b/Lib/test/test_free_threading/test_heapq.py @@ -30,7 +30,7 @@ def test_racing_heapify(self): shuffle(heap) run_concurrently( - worker_func=heapq.heapify, args=(heap,), nthreads=NTHREADS + worker_func=heapq.heapify, nthreads=NTHREADS, args=(heap,) ) self.test_heapq.check_invariant(heap) @@ -42,7 +42,7 @@ def heappush_func(heap): heapq.heappush(heap, item) run_concurrently( - worker_func=heappush_func, args=(heap,), nthreads=NTHREADS + worker_func=heappush_func, nthreads=NTHREADS, args=(heap,) ) self.test_heapq.check_invariant(heap) @@ -64,8 +64,8 @@ def heappop_func(heap, pop_count): run_concurrently( worker_func=heappop_func, - args=(heap, per_thread_pop_count), nthreads=NTHREADS, + args=(heap, per_thread_pop_count), ) self.assertEqual(len(heap), 0) @@ -80,8 +80,8 @@ def heappushpop_func(heap, pushpop_items): run_concurrently( worker_func=heappushpop_func, - args=(heap, pushpop_items), nthreads=NTHREADS, + args=(heap, pushpop_items), ) self.assertEqual(len(heap), OBJECT_COUNT) self.test_heapq.check_invariant(heap) @@ -96,8 +96,8 @@ def heapreplace_func(heap, replace_items): run_concurrently( worker_func=heapreplace_func, - args=(heap, replace_items), nthreads=NTHREADS, + args=(heap, replace_items), ) self.assertEqual(len(heap), OBJECT_COUNT) self.test_heapq.check_invariant(heap) @@ -107,7 +107,7 @@ def test_racing_heapify_max(self): shuffle(max_heap) run_concurrently( - worker_func=heapq.heapify_max, args=(max_heap,), nthreads=NTHREADS + worker_func=heapq.heapify_max, nthreads=NTHREADS, args=(max_heap,) ) self.test_heapq.check_max_invariant(max_heap) @@ -119,7 +119,7 @@ def heappush_max_func(max_heap): heapq.heappush_max(max_heap, item) run_concurrently( - worker_func=heappush_max_func, args=(max_heap,), nthreads=NTHREADS + worker_func=heappush_max_func, nthreads=NTHREADS, args=(max_heap,) ) self.test_heapq.check_max_invariant(max_heap) @@ -141,8 +141,8 @@ def heappop_max_func(max_heap, pop_count): run_concurrently( worker_func=heappop_max_func, - args=(max_heap, per_thread_pop_count), nthreads=NTHREADS, + args=(max_heap, per_thread_pop_count), ) self.assertEqual(len(max_heap), 0) @@ -157,8 +157,8 @@ def heappushpop_max_func(max_heap, pushpop_items): run_concurrently( worker_func=heappushpop_max_func, - args=(max_heap, pushpop_items), nthreads=NTHREADS, + args=(max_heap, pushpop_items), ) self.assertEqual(len(max_heap), OBJECT_COUNT) self.test_heapq.check_max_invariant(max_heap) @@ -173,8 +173,8 @@ def heapreplace_max_func(max_heap, replace_items): run_concurrently( worker_func=heapreplace_max_func, - args=(max_heap, replace_items), nthreads=NTHREADS, + args=(max_heap, replace_items), ) self.assertEqual(len(max_heap), OBJECT_COUNT) self.test_heapq.check_max_invariant(max_heap) @@ -204,7 +204,7 @@ def worker(): except IndexError: pass - run_concurrently(worker, (), n_threads * 2) + run_concurrently(worker, n_threads * 2) @staticmethod def is_sorted_ascending(lst): From f51d0d9217caef9c996536f6a932fa9ca35533fb Mon Sep 17 00:00:00 2001 From: alperyoney Date: Thu, 3 Jul 2025 11:11:44 -0700 Subject: [PATCH 6/9] gh-116738: Consolidate group function calls under one mutex --- Modules/grpmodule.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/Modules/grpmodule.c b/Modules/grpmodule.c index b33f5e472dee98..652958618a2c4c 100644 --- a/Modules/grpmodule.c +++ b/Modules/grpmodule.c @@ -55,6 +55,11 @@ get_grp_state(PyObject *module) static struct PyModuleDef grpmodule; +/* Mutex to protect calls to getgrgid(), getgrnam(), and getgrent(). + * These functions return pointer to static data structure, which + * may be overwritten by any subsequent calls. */ +static PyMutex group_db_mutex = {0}; + #define DEFAULT_BUFFER_SIZE 1024 static PyObject * @@ -168,15 +173,14 @@ grp_getgrgid_impl(PyObject *module, PyObject *id) Py_END_ALLOW_THREADS #else - static PyMutex getgrgid_mutex = {0}; - PyMutex_Lock(&getgrgid_mutex); + PyMutex_Lock(&group_db_mutex); // The getgrgid() function need not be thread-safe. // https://pubs.opengroup.org/onlinepubs/9699919799/functions/getgrgid.html p = getgrgid(gid); #endif if (p == NULL) { #ifndef HAVE_GETGRGID_R - PyMutex_Unlock(&getgrgid_mutex); + PyMutex_Unlock(&group_db_mutex); #endif PyMem_RawFree(buf); if (nomem == 1) { @@ -193,7 +197,7 @@ grp_getgrgid_impl(PyObject *module, PyObject *id) #ifdef HAVE_GETGRGID_R PyMem_RawFree(buf); #else - PyMutex_Unlock(&getgrgid_mutex); + PyMutex_Unlock(&group_db_mutex); #endif return retval; } @@ -258,15 +262,14 @@ grp_getgrnam_impl(PyObject *module, PyObject *name) Py_END_ALLOW_THREADS #else - static PyMutex getgrnam_mutex = {0}; - PyMutex_Lock(&getgrnam_mutex); + PyMutex_Lock(&group_db_mutex); // The getgrnam() function need not be thread-safe. // https://pubs.opengroup.org/onlinepubs/9699919799/functions/getgrnam.html p = getgrnam(name_chars); #endif if (p == NULL) { #ifndef HAVE_GETGRNAM_R - PyMutex_Unlock(&getgrnam_mutex); + PyMutex_Unlock(&group_db_mutex); #endif if (nomem == 1) { PyErr_NoMemory(); @@ -278,7 +281,7 @@ grp_getgrnam_impl(PyObject *module, PyObject *name) } retval = mkgrent(module, p); #ifndef HAVE_GETGRNAM_R - PyMutex_Unlock(&getgrnam_mutex); + PyMutex_Unlock(&group_db_mutex); #endif out: PyMem_RawFree(buf); @@ -304,8 +307,7 @@ grp_getgrall_impl(PyObject *module) return NULL; } - static PyMutex getgrall_mutex = {0}; - PyMutex_Lock(&getgrall_mutex); + PyMutex_Lock(&group_db_mutex); setgrent(); struct group *p; @@ -325,7 +327,7 @@ grp_getgrall_impl(PyObject *module) done: endgrent(); - PyMutex_Unlock(&getgrall_mutex); + PyMutex_Unlock(&group_db_mutex); return d; } From 79028c803c30e359e9d6c35636e32f895bd7fbce Mon Sep 17 00:00:00 2001 From: alperyoney Date: Wed, 9 Jul 2025 10:16:36 -0700 Subject: [PATCH 7/9] gh-116738: Add global mutex variable to lint ignore list --- Tools/c-analyzer/cpython/ignored.tsv | 1 + 1 file changed, 1 insertion(+) diff --git a/Tools/c-analyzer/cpython/ignored.tsv b/Tools/c-analyzer/cpython/ignored.tsv index 15b18f5286b399..64a9f11a944176 100644 --- a/Tools/c-analyzer/cpython/ignored.tsv +++ b/Tools/c-analyzer/cpython/ignored.tsv @@ -167,6 +167,7 @@ Python/sysmodule.c - _preinit_xoptions - # XXX need race protection? Modules/faulthandler.c faulthandler_dump_traceback reentrant - Modules/faulthandler.c faulthandler_dump_c_stack reentrant - +Modules/grpmodule.c - group_db_mutex - Python/pylifecycle.c _Py_FatalErrorFormat reentrant - Python/pylifecycle.c fatal_error reentrant - From 68567529674a8d9368fab451d04b4c7721c8d529 Mon Sep 17 00:00:00 2001 From: alperyoney Date: Fri, 11 Jul 2025 12:53:47 -0700 Subject: [PATCH 8/9] gh-116738: Convert generator to list comprehension --- Lib/test/support/threading_helper.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/support/threading_helper.py b/Lib/test/support/threading_helper.py index aa896841229508..3e04c344a0d66f 100644 --- a/Lib/test/support/threading_helper.py +++ b/Lib/test/support/threading_helper.py @@ -262,10 +262,10 @@ def wrapper_func(*args, **kwargs): worker_func(*args, **kwargs) with catch_threading_exception() as cm: - workers = ( + workers = [ threading.Thread(target=wrapper_func, args=args, kwargs=kwargs) for _ in range(nthreads) - ) + ] with start_threads(workers): pass From ed89087966bfa9c743470654041233399dcad377 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Sun, 13 Jul 2025 11:13:16 +0530 Subject: [PATCH 9/9] Update Misc/NEWS.d/next/Core_and_Builtins/2025-06-12-00-03-34.gh-issue-116738.iBBAdo.rst --- .../2025-06-12-00-03-34.gh-issue-116738.iBBAdo.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-06-12-00-03-34.gh-issue-116738.iBBAdo.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-12-00-03-34.gh-issue-116738.iBBAdo.rst index 1f3c53c3464ace..2a1ed2944d8ddf 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2025-06-12-00-03-34.gh-issue-116738.iBBAdo.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-12-00-03-34.gh-issue-116738.iBBAdo.rst @@ -1 +1 @@ -Make methods in :mod:`grp` thread-safe on the :term:`free threaded ` build. +Make functions in :mod:`grp` thread-safe on the :term:`free threaded ` build.