8000 gh-129701: Fix a data race in `intern_common` in the free threaded build · python/cpython@a12edba · GitHub
[go: up one dir, main page]

Skip to content

Commit a12edba

Browse files
committed
gh-129701: Fix a data race in intern_common in the free threaded build
* Use a mutex to avoid potentially returning a non-immortalized string, because immortalization happens after the insertion into the interned dict. * Use `Py_DECREF()` calls instead of `Py_SET_REFCNT(s, Py_REFCNT(s) - 2)` for thread-safety. This code path isn't performance sensistive, so just use `Py_DECREF()` unconditionally for simplicity.
1 parent 0559339 commit a12edba

File tree

3 files changed

+18
-6
lines changed

3 files changed

+18
-6
lines changed

Include/internal/pycore_global_objects.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ struct _Py_static_objects {
6464
(interp)->cached_objects.NAME
6565

6666
struct _Py_interp_cached_objects {
67+
#ifdef Py_GIL_DISABLED
68+
PyMutex interned_mutex;
69+
#endif
6770
PyObject *interned_strings;
6871

6972
/* object.__reduce__ */

Lib/test/test_exceptions.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1484,6 +1484,7 @@ def test_recursion_normalizing_infinite_exception(self):
14841484
self.assertIn(b'Done.', out)
14851485

14861486

1487+
@support.thread_unsafe("sys.setrecursionlimit")
14871488
def test_recursion_in_except_handler(self):
14881489

14891490
def set_relative_recursion_limit(n):

Objects/unicodeobject.c

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,14 @@ NOTE: In the interpreter's initialization phase, some globals are currently
112112
# define _PyUnicode_CHECK(op) PyUnicode_Check(op)
113113
#endif
114114

115+
#ifdef Py_GIL_DISABLED
116+
# define LOCK_INTERNED(interp) PyMutex_Lock(&_Py_INTERP_CACHED_OBJECT(interp, interned_mutex))
117+
# define UNLOCK_INTERNED(interp) PyMutex_Unlock(&_Py_INTERP_CACHED_OBJECT(interp, interned_mutex))
118+
#else
119+
# define LOCK_INTERNED(interp)
120+
# define UNLOCK_INTERNED(interp)
121+
#endif
122+
115123
static inline char* _PyUnicode_UTF8(PyObject *op)
116124
{
117125
return FT_ATOMIC_LOAD_PTR_ACQUIRE(_PyCompactUnicodeObject_CAST(op)->utf8);
@@ -15815,11 +15823,13 @@ intern_common(PyInterpreterState *interp, PyObject *s /* stolen */,
1581515823
PyObject *interned = get_interned_dict(interp);
1581615824
assert(interned != NULL);
1581715825

15826+
LOCK_INTERNED(interp);
1581815827
PyObject *t;
1581915828
{
1582015829
int res = PyDict_SetDefaultRef(interned, s, s, &t);
1582115830
if (res < 0) {
1582215831
PyErr_Clear();
15832+
UNLOCK_INTERNED(interp);
1582315833
return s;
1582415834
}
1582515835
else if (res == 1) {
@@ -15829,6 +15839,7 @@ intern_common(PyInterpreterState *interp, PyObject *s /* stolen */,
1582915839
PyUnicode_CHECK_INTERNED(t) == SSTATE_INTERNED_MORTAL) {
1583015840
immortalize_interned(t);
1583115841
}
15842+
UNLOCK_INTERNED(interp);
1583215843
return t;
1583315844
}
1583415845
else {
@@ -15845,12 +15856,8 @@ intern_common(PyInterpreterState *interp, PyObject *s /* stolen */,
1584515856
if (!_Py_IsImmortal(s)) {
1584615857
/* The two references in interned dict (key and value) are not counted.
1584715858
unicode_dealloc() and _PyUnicode_ClearInterned() take care of this. */
15848-
Py_SET_REFCNT(s, Py_REFCNT(s) - 2);
15849-
#ifdef Py_REF_DEBUG
15850-
/* let's be pedantic with the ref total */
15851-
_Py_DecRefTotal(_PyThreadState_GET());
15852-
_Py_DecRefTotal(_PyThreadState_GET());
15853-
#endif
15859+
Py_DECREF(s);
15860+
Py_DECREF(s);
1585415861
}
1585515862
FT_ATOMIC_STORE_UINT16_RELAXED(_PyUnicode_STATE(s).interned, SSTATE_INTERNED_MORTAL);
1585615863

@@ -15865,6 +15872,7 @@ intern_common(PyInterpreterState *interp, PyObject *s /* stolen */,
1586515872
immortalize_interned(s);
1586615873
}
1586715874

15875+
UNLOCK_INTERNED(interp);
1586815876
return s;
1586915877
}
1587015878

0 commit comments

Comments
 (0)
0