Skip to content

Commit 4572903

Browse files
tonghuarootwarsaw
andauthored
gh-153056: Fix a data race compiling the string.Template pattern in free-threading builds (#153057)
* gh-153056: Fix a data race compiling the string.Template pattern in free-threading builds Template compiles its substitution pattern lazily and caches it on the class. On the free-threaded build two concurrent first uses could race: a thread that observed the pattern another thread had just compiled would try to recompile it, and re.compile() rejects flags on an already-compiled pattern, raising a spurious ValueError. Return the already-compiled pattern instead. As a side effect, a subclass that supplies an already-compiled pattern now works too; previously it raised the same ValueError at class definition. * Trim test comments and NEWS wording * Document that the pattern attribute accepts a string or a compiled regex * Comment the three states of pattern and note the documented-behavior fix in NEWS * Update Doc/library/string.rst --------- Co-authored-by: Barry Warsaw <barry@python.org>
1 parent aa533dc commit 4572903

5 files changed

Lines changed: 62 additions & 1 deletion

File tree

Doc/library/string.rst

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -971,7 +971,8 @@ attributes:
971971

972972
Alternatively, you can provide the entire regular expression pattern by
973973
overriding the class attribute *pattern*. If you do this, the value must be a
974-
regular expression object with four named capturing groups. The capturing
974+
regular expression pattern string, or a compiled regular expression
975+
object, with four named capturing groups. The capturing
975976
groups correspond to the rules given above, along with the invalid placeholder
976977
rule:
977978

Lib/string/__init__.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,14 @@ def __init_subclass__(cls):
8383
def _compile_pattern(cls):
8484
import re # deferred import, for performance
8585

86+
# `pattern` may be the `_TemplatePattern` sentinel (not yet compiled), an
87+
# already-compiled regular expression object (as documented), or a string
88+
# regular expression. An already-compiled object is returned as-is; the
89+
# other two are compiled and cached back on the class.
8690
pattern = cls.__dict__.get('pattern', _TemplatePattern)
91+
if isinstance(pattern, re.Pattern):
92+
# re.compile() rejects flags on an already-compiled pattern.
93+
return pattern
8794
if pattern is _TemplatePattern:
8895
delim = re.escape(cls.delimiter)
8996
id = cls.idpattern
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import string
2+
import unittest
3+
from string import Template
4+
5+
from test.support import threading_helper
6+
7+
8+
@threading_helper.requires_working_threading()
9+
class TestTemplateCompileRace(unittest.TestCase):
10+
def test_concurrent_first_use(self):
11+
# Racing the lazy pattern compile must not raise a spurious ValueError
12+
# from recompiling an already-compiled pattern. A throwaway subclass,
13+
# re-armed to the sentinel each round, keeps string.Template unmutated
14+
# (subclasses precompile eagerly in __init_subclass__).
15+
uncompiled = string._TemplatePattern
16+
errors = []
17+
18+
def use_template(cls):
19+
try:
20+
cls("$x and ${y}").substitute(x=1, y=2)
21+
except Exception as e:
22+
errors.append(e)
23+
24+
for _ in range(20):
25+
class T(Template):
26+
pass
27+
T.pattern = uncompiled
28+
T.flags = None
29+
threading_helper.run_concurrently(use_template, nthreads=10, args=(T,))
30+
31+
self.assertEqual(errors, [], msg=f"unexpected errors: {errors}")
32+
33+
34+
if __name__ == "__main__":
35+
unittest.main()

Lib/test/test_string/test_string.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,20 @@ def test_SafeTemplate(self):
299299
eq(s.safe_substitute(dict(who='tim', what='ham', meal='dinner')),
300300
'tim likes ham for dinner')
301301

302+
def test_precompiled_pattern(self):
303+
# A subclass may supply an already-compiled pattern; it must be reused,
304+
# not recompiled (re.compile() rejects flags on a compiled pattern).
305+
import re
306+
compiled = re.compile(
307+
r'\$(?:(?P<escaped>\$)|(?P<named>[a-z]+)|'
308+
r'\{(?P<braced>[a-z]+)\}|(?P<invalid>))')
309+
class MyTemplate(Template):
310+
pattern = compiled
311+
self.assertIs(MyTemplate.pattern, compiled)
312+
self.assertEqual(
313+
MyTemplate('$who likes $what').substitute(who='tim', what='ham'),
314+
'tim likes ham')
315+
302316
def test_invalid_placeholders(self):
303317
raises = self.assertRaises
304318
s = Template('$who likes $')
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix :class:`string.Template` raising a spurious :exc:`ValueError` when the
2+
*pattern* attribute is a compiled regular expression object, which the
3+
documentation allows. On the free-threaded build this also occurred as a data
4+
race on the first concurrent use.

0 commit comments

Comments
 (0)