Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
1 change: 1 addition & 0 deletions Lib/test/libregrtest/tsan.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
TSAN_PARALLEL_TESTS = [
'test_abc',
'test_hashlib',
'test_re',
]


Expand Down
13 changes: 13 additions & 0 deletions Lib/test/test_re.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from test import support
from test.support import (gc_collect, bigmemtest, _2G,
cpython_only, captured_stdout,
check_disallow_instantiation, is_emscripten, is_wasi,
Expand Down Expand Up @@ -33,6 +34,7 @@ class B(bytes):
def __getitem__(self, index):
return B(super().__getitem__(index))

@support.skip_if_sanitizer("not everything here is free-threading safe yet", thread=True)
class ReTests(unittest.TestCase):

def assertTypedEqual(self, actual, expect, msg=None):
Expand Down Expand Up @@ -2714,6 +2716,7 @@ def get_debug_out(pat):
return out.getvalue()


@support.skip_if_sanitizer("not everything here is free-threading safe yet", thread=True)
@cpython_only
class DebugTests(unittest.TestCase):
maxDiff = None
Expand Down Expand Up @@ -2810,6 +2813,7 @@ def test_possesive_repeat(self):
''')


@support.skip_if_sanitizer("not everything here is free-threading safe yet", thread=True)
class PatternReprTests(unittest.TestCase):
def check(self, pattern, expected):
self.assertEqual(repr(re.compile(pattern)), expected)
Expand Down Expand Up @@ -2890,6 +2894,7 @@ def test_flags_repr(self):
"re.ASCII|re.LOCALE|re.UNICODE|re.MULTILINE|re.DEBUG|0xffe01")


@support.skip_if_sanitizer("not everything here is free-threading safe yet", thread=True)
class ImplementationTest(unittest.TestCase):
"""
Test implementation details of the re module.
Expand Down Expand Up @@ -3027,6 +3032,7 @@ def test_sre_template_invalid_group_index(self):
self.assertIn("an integer is required", str(cm.exception))


@support.skip_if_sanitizer("not everything here is free-threading safe yet", thread=True)
class ExternalTests(unittest.TestCase):

def test_re_benchmarks(self):
Expand Down Expand Up @@ -3140,5 +3146,12 @@ def test_re_tests(self):
self.assertTrue(obj.search(s))


@unittest.skipUnless(support.check_sanitizer(thread=True), 'only has meaning if TSAN enabled')
class FreeThreadingTests(unittest.TestCase):
def test_compile_pattern(self):
# gh-129983: Data race in compile_template in sre.c
re.sub(r"(\d+)", r"\1kg", "Weight: 50")


if __name__ == "__main__":
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix data race in compile_template in :file:`sre.c`.
10 changes: 9 additions & 1 deletion Modules/_sre/sre.c
Original file line number Diff line number Diff line change
Expand Up @@ -1167,13 +1167,21 @@ compile_template(_sremodulestate *module_state,
PatternObject *pattern, PyObject *template)
{
/* delegate to Python code */
PyObject *func = module_state->compile_template;
PyObject *func = FT_ATOMIC_LOAD_PTR_RELAXED(module_state->compile_template);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use FT_ATOMIC_LOAD_PTR here.

FT_ATOMIC_LOAD_PTR_ACQUIRE would probably also be fine, but I think symmetry between the load and store is generally a good idea.

if (func == NULL) {
func = PyImport_ImportModuleAttrString("re", "_compile_template");
if (func == NULL) {
return NULL;
}
#ifdef Py_GIL_DISABLED
PyObject *other_func = NULL;
if (!_Py_atomic_compare_exchange_ptr(&module_state->compile_template, &other_func, func)) {
Py_DECREF(func);
func = other_func;
}
#else
Py_XSETREF(module_state->compile_template, func);
#endif
}

PyObject *args[] = {(PyObject *)pattern, template};
Expand Down
Loading