diff --git a/Lib/test/test_bytes.py b/Lib/test/test_bytes.py index 7ca38bb8c8421e..a6cf899fa51e75 100644 --- a/Lib/test/test_bytes.py +++ b/Lib/test/test_bytes.py @@ -2655,6 +2655,10 @@ def zfill(b, a): c = a.zfill(0x400000) assert not c or c[-1] not in (0xdd, 0xcd) + def resize(b, a): # MODIFIES! + b.wait() + a.resize(10) + def take_bytes(b, a): # MODIFIES! b.wait() c = a.take_bytes() @@ -2728,6 +2732,8 @@ def check(funcs, a=None, *args): check([clear] + [startswith] * 10) check([clear] + [strip] * 10) + check([clear] + [resize] * 10) + check([clear] + [take_bytes] * 10) check([take_bytes_n] * 10, bytearray(b'0123456789' * 0x400)) check([take_bytes_n] * 10, bytearray(b'0123456789' * 5)) diff --git a/Lib/test/test_free_threading/test_capi.py b/Lib/test/test_free_threading/test_capi.py new file mode 100644 index 00000000000000..146d7cfc97adb7 --- /dev/null +++ b/Lib/test/test_free_threading/test_capi.py @@ -0,0 +1,47 @@ +import ctypes +import sys +import unittest + +from test.support import threading_helper +from test.support.threading_helper import run_concurrently + + +_PyImport_AddModuleRef = ctypes.pythonapi.PyImport_AddModuleRef +_PyImport_AddModuleRef.argtypes = (ctypes.c_char_p,) +_PyImport_AddModuleRef.restype = ctypes.py_object + + +@threading_helper.requires_working_threading() +class TestImportCAPI(unittest.TestCase): + def test_pyimport_addmoduleref_thread_safe(self): + # gh-137422: Concurrent calls to PyImport_AddModuleRef with the same + # module name must return the same module object. + + NUM_ITERS = 10 + NTHREADS = 4 + + module_name = f"test_free_threading_addmoduleref_{id(self)}" + module_name_bytes = module_name.encode() + sys.modules.pop(module_name, None) + results = [] + + def worker(): + module = _PyImport_AddModuleRef(module_name_bytes) + results.append(module) + + for _ in range(NUM_ITERS): + try: + run_concurrently(worker_func=worker, nthreads=NTHREADS) + self.assertEqual(len(results), NTHREADS) + reference = results[0] + for module in results[1:]: + self.assertIs(module, reference) + self.assertIn(module_name, sys.modules) + self.assertIs(sys.modules[module_name], reference) + finally: + results.clear() + sys.modules.pop(module_name, None) + + +if __name__ == "__main__": + unittest.main() diff --git a/Lib/test/test_profiling/test_sampling_profiler/test_integration.py b/Lib/test/test_profiling/test_sampling_profiler/test_integration.py index e1c80fa6d5d1b7..4ba18355fc104a 100644 --- a/Lib/test/test_profiling/test_sampling_profiler/test_integration.py +++ b/Lib/test/test_profiling/test_sampling_profiler/test_integration.py @@ -27,6 +27,7 @@ requires_subprocess, captured_stdout, captured_stderr, + SHORT_TIMEOUT, ) from .helpers import ( @@ -37,6 +38,9 @@ ) from .mocks import MockFrameInfo, MockThreadInfo, MockInterpreterInfo +# Duration for profiling tests - long enough for process to complete naturally +PROFILING_TIMEOUT = str(int(SHORT_TIMEOUT)) + @skip_if_not_supported @unittest.skipIf( @@ -381,47 +385,15 @@ def cpu_intensive_work(): result = result % 1000000 return result -def medium_computation(): - """Medium complexity function.""" - result = 0 - for i in range(100): - result += i * i - return result - -def fast_loop(): - """Fast simple loop.""" - total = 0 - for i in range(50): - total += i - return total - -def nested_calls(): - """Test nested function calls.""" - def level1(): - def level2(): - return medium_computation() - return level2() - return level1() - def main_loop(): - """Main test loop with different execution paths.""" - iteration = 0 - - while True: - iteration += 1 + """Main test loop.""" + max_iterations = 200 - # Different execution paths - focus on CPU intensive work - if iteration % 3 == 0: - # Very CPU intensive - result = cpu_intensive_work() - elif iteration % 2 == 0: - # Expensive recursive operation (increased frequency for slower machines) - result = slow_fibonacci(12) + for iteration in range(max_iterations): + if iteration % 2 == 0: + result = slow_fibonacci(15) else: - # Medium operation - result = nested_calls() - - # No sleep - keep CPU busy + result = cpu_intensive_work() if __name__ == "__main__": main_loop() @@ -434,9 +406,10 @@ def test_sampling_basic_functionality(self): mock.patch("sys.stdout", captured_output), ): try: + # Sample for up to SHORT_TIMEOUT seconds, but process exits after fixed iterations profiling.sampling.sample.sample( subproc.process.pid, - duration_sec=2, + duration_sec=SHORT_TIMEOUT, sample_interval_usec=1000, # 1ms show_summary=False, ) @@ -578,7 +551,8 @@ def test_sample_target_script(self): script_file.flush() self.addCleanup(close_and_unlink, script_file) - test_args = ["profiling.sampling.sample", "-d", "1", script_file.name] + # Sample for up to SHORT_TIMEOUT seconds, but process exits after fixed iterations + test_args = ["profiling.sampling.sample", "-d", PROFILING_TIMEOUT, script_file.name] with ( mock.patch("sys.argv", test_args), @@ -612,7 +586,7 @@ def test_sample_target_module(self): test_args = [ "profiling.sampling.sample", "-d", - "1", + PROFILING_TIMEOUT, "-m", "test_module", ] diff --git a/Misc/NEWS.d/next/C_API/2025-11-21-10-34-00.gh-issue-137422.tzZKLi.rst b/Misc/NEWS.d/next/C_API/2025-11-21-10-34-00.gh-issue-137422.tzZKLi.rst new file mode 100644 index 00000000000000..656289663cfebb --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2025-11-21-10-34-00.gh-issue-137422.tzZKLi.rst @@ -0,0 +1,4 @@ +Fix :term:`free threading` race condition in +:c:func:`PyImport_AddModuleRef`. It was previously possible for two calls to +the function return two different objects, only one of which was stored in +:data:`sys.modules`. diff --git a/Python/import.c b/Python/import.c index 9ab2d3b3552235..de183de44da843 100644 --- a/Python/import.c +++ b/Python/import.c @@ -3,6 +3,7 @@ #include "Python.h" #include "pycore_audit.h" // _PySys_Audit() #include "pycore_ceval.h" +#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION() #include "pycore_hashtable.h" // _Py_hashtable_new_full() #include "pycore_import.h" // _PyImport_BootstrapImp() #include "pycore_initconfig.h" // _PyStatus_OK() @@ -309,13 +310,8 @@ PyImport_GetModule(PyObject *name) if not, create a new one and insert it in the modules dictionary. */ static PyObject * -import_add_module(PyThreadState *tstate, PyObject *name) +import_add_module_lock_held(PyObject *modules, PyObject *name) { - PyObject *modules = get_modules_dict(tstate, false); - if (modules == NULL) { - return NULL; - } - PyObject *m; if (PyMapping_GetOptionalItem(modules, name, &m) < 0) { return NULL; @@ -335,6 +331,21 @@ import_add_module(PyThreadState *tstate, PyObject *name) return m; } +static PyObject * +import_add_module(PyThreadState *tstate, PyObject *name) +{ + PyObject *modules = get_modules_dict(tstate, false); + if (modules == NULL) { + return NULL; + } + + PyObject *m; + Py_BEGIN_CRITICAL_SECTION(modules); + m = import_add_module_lock_held(modules, name); + Py_END_CRITICAL_SECTION(); + return m; +} + PyObject * PyImport_AddModuleRef(const char *name) {