From 3ded9f021892ca3d3bffedc289ade61e4e8041e2 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 6 Feb 2025 17:06:47 +0000 Subject: [PATCH 1/3] gh-129694: Add test_exceptions to parallel TSAN jobs This would catch the data race involving the MemoryError freelist that was fixed in gh-129668. --- Lib/test/libregrtest/tsan.py | 1 + Lib/test/support/__init__.py | 2 +- Lib/test/test_exceptions.py | 10 ++++++++++ Tools/tsan/suppressions_free_threading.txt | 7 +++++++ 4 files changed, 19 insertions(+), 1 deletion(-) diff --git a/Lib/test/libregrtest/tsan.py b/Lib/test/libregrtest/tsan.py index 1b32deec12bd75..dbf4c8cfe3f196 100644 --- a/Lib/test/libregrtest/tsan.py +++ b/Lib/test/libregrtest/tsan.py @@ -33,6 +33,7 @@ # the regression test runner with the `--parallel-threads` option enabled. TSAN_PARALLEL_TESTS = [ 'test_abc', + 'test_exceptions', ] diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index f31d98bf731d67..952e6c238b9a05 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -2870,7 +2870,7 @@ def force_not_colorized(func): def wrapper(*args, **kwargs): with no_color(): return func(*args, **kwargs) - return wrapper + return thread_unsafe(wrapper) # modifying the environment is thread-unsafe def force_not_colorized_test_class(cls): diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index 3838eb5b27c9e6..c4b3a29e6e98fc 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -60,6 +60,7 @@ def raise_catch(self, exc, excname): self.assertEqual(buf1, buf2) self.assertEqual(exc.__name__, excname) + @support.thread_unsafe("TESTFN") def testRaising(self): self.raise_catch(AttributeError, "AttributeError") self.assertRaises(AttributeError, getattr, sys, "undefined_attribute") @@ -1361,6 +1362,7 @@ def test_unicode_error_str_does_not_crash(self): self.assertIsInstance(str(exc), str) @no_tracing + @support.thread_unsafe("captures stderr") def test_badisinstance(self): # Bug #2542: if issubclass(e, MyException) raises an exception, # it should be ignored @@ -1484,6 +1486,7 @@ def test_recursion_normalizing_infinite_exception(self): self.assertIn(b'Done.', out) + @support.thread_unsafe("uses sys.setrecursionlimit") def test_recursion_in_except_handler(self): def set_relative_recursion_limit(n): @@ -1619,6 +1622,7 @@ class C(object): @cpython_only @unittest.skipIf(_testcapi is None, "requires _testcapi") + @support.thread_unsafe("gc_collect()") def test_memory_error_cleanup(self): # Issue #5437: preallocated MemoryError instances should not keep # traceback objects alive. @@ -1668,6 +1672,7 @@ def test_errno_ENOTDIR(self): os.listdir(__file__) self.assertEqual(cm.exception.errno, errno.ENOTDIR, cm.exception) + @support.thread_unsafe("uses catch_unraisable_exception") def test_unraisable(self): # Issue #22836: PyErr_WriteUnraisable() should give sensible reports class BrokenDel: @@ -1687,6 +1692,7 @@ def __del__(self): f"deallocator {obj_repr}") self.assertIsNotNone(cm.unraisable.exc_traceback) + @support.thread_unsafe("captures stderr") def test_unhandled(self): # Check for sensible reporting of unhandled exceptions for exc_type in (ValueError, BrokenStrException): @@ -1786,6 +1792,7 @@ def g(): next(i) @unittest.skipUnless(__debug__, "Won't work if __debug__ is False") + @support.thread_unsafe("modifies global AssertionError") def test_assert_shadowing(self): # Shadowing AssertionError would cause the assert statement to # misbehave. @@ -1877,6 +1884,7 @@ def test_name_error_has_name(self): except NameError as exc: self.assertEqual("bluch", exc.name) + @support.thread_unsafe("captures stderr") def test_issue45826(self): # regression test for bpo-45826 def f(): @@ -1893,6 +1901,7 @@ def f(): self.assertIn("aab", err.getvalue()) + @support.thread_unsafe("captures stderr") def test_issue45826_focused(self): def f(): try: @@ -2295,6 +2304,7 @@ class MySyntaxError(SyntaxError): ^^^^^ """, err.getvalue()) + @support.thread_unsafe("TESTFN") def test_encodings(self): self.addCleanup(unlink, TESTFN) source = ( diff --git a/Tools/tsan/suppressions_free_threading.txt b/Tools/tsan/suppressions_free_threading.txt index e5eb665ae212de..330bfb382ba56b 100644 --- a/Tools/tsan/suppressions_free_threading.txt +++ b/Tools/tsan/suppressions_free_threading.txt @@ -44,5 +44,12 @@ race_top:PyThreadState_Clear # Only seen on macOS, sample: https://gist.github.com/aisk/dda53f5d494a4556c35dde1fce03259c race_top:set_default_allocator_unlocked +# gh-129701: data race during modification of refcount when interning strings +race:intern_common + +# gh-128130: race on _PyRuntime.signals.unhandled_keyboard_interrupt +race_top:_PyErr_Display +race_top:run_eval_code_obj + # https://gist.github.com/mpage/6962e8870606cfc960e159b407a0cb40 thread:pthread_create From a429803b9d230818bee49f5f2a1df5e4593c6bf9 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 6 Feb 2025 19:40:21 +0000 Subject: [PATCH 2/3] Only access interp->sys_tracing_threads within lock --- Python/legacy_tracing.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/legacy_tracing.c b/Python/legacy_tracing.c index 97634f9183c7d5..a80622a92c41f6 100644 --- a/Python/legacy_tracing.c +++ b/Python/legacy_tracing.c @@ -522,6 +522,7 @@ _PyEval_SetProfile(PyThreadState *tstate, Py_tracefunc func, PyObject *arg) static Py_ssize_t setup_tracing(PyThreadState *tstate, Py_tracefunc func, PyObject *arg, PyObject **old_traceobj) { + assert(tstate->interp->sys_tracing_threads >= 0); *old_traceobj = NULL; /* Setup PEP 669 monitoring callbacks and events. */ if (!tstate->interp->sys_trace_initialized) { @@ -595,7 +596,6 @@ _PyEval_SetTrace(PyThreadState *tstate, Py_tracefunc func, PyObject *arg) if (_PySys_Audit(current_tstate, "sys.settrace", NULL) < 0) { return -1; } - assert(tstate->interp->sys_tracing_threads >= 0); // needs to be decref'd outside of the lock PyObject *old_traceobj; LOCK_SETUP(); From 0234e5bacb61f6850a29239cd939689ef8078b2d Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 6 Feb 2025 20:10:14 +0000 Subject: [PATCH 3/3] Warnings context manager is not thread-safe --- Lib/test/test_exceptions.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index c4b3a29e6e98fc..f9abce19460fe1 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -2022,6 +2022,7 @@ def test_reset_attributes(self): self.assertEqual(exc.name, None) self.assertEqual(exc.path, None) + @support.thread_unsafe("check_warnings") def test_non_str_argument(self): # Issue #15778 with check_warnings(('', BytesWarning), quiet=True):