Skip to content

Commit c743314

Browse files
[3.13] gh-127020: Make PyCode_GetCode thread-safe for free threading (GH-127043) (GH-127107)
Some fields in PyCodeObject are lazily initialized. Use atomics and critical sections to make their initializations and accesses thread-safe. (cherry picked from commit 3926842) Co-authored-by: Sam Gross <[email protected]>
1 parent c09366b commit c743314

File tree

3 files changed

+85
-27
lines changed

3 files changed

+85
-27
lines changed
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import unittest
2+
3+
from threading import Thread
4+
from unittest import TestCase
5+
6+
from test.support import threading_helper
7+
8+
@threading_helper.requires_working_threading()
9+
class TestCode(TestCase):
10+
def test_code_attrs(self):
11+
"""Test concurrent accesses to lazily initialized code attributes"""
12+
code_objects = []
13+
for _ in range(1000):
14+
code_objects.append(compile("a + b", "<string>", "eval"))
15+
16+
def run_in_thread():
17+
for code in code_objects:
18+
self.assertIsInstance(code.co_code, bytes)
19+
self.assertIsInstance(code.co_freevars, tuple)
20+
self.assertIsInstance(code.co_varnames, tuple)
21+
22+
threads = [Thread(target=run_in_thread) for _ in range(2)]
23+
for thread in threads:
24+
thread.start()
25+
for thread in threads:
26+
thread.join()
27+
28+
29+
if __name__ == "__main__":
30+
unittest.main()
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix a crash in the free threading build when :c:func:`PyCode_GetCode`,
2+
:c:func:`PyCode_GetVarnames`, :c:func:`PyCode_GetCellvars`, or
3+
:c:func:`PyCode_GetFreevars` were called from multiple threads at the same
4+
time.

Objects/codeobject.c

Lines changed: 51 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -304,21 +304,32 @@ validate_and_copy_tuple(PyObject *tup)
304304
}
305305

306306
static int
307-
init_co_cached(PyCodeObject *self) {
308-
if (self->_co_cached == NULL) {
309-
self->_co_cached = PyMem_New(_PyCoCached, 1);
310-
if (self->_co_cached == NULL) {
307+
init_co_cached(PyCodeObject *self)
308+
{
309+
_PyCoCached *cached = FT_ATOMIC_LOAD_PTR(self->_co_cached);
310+
if (cached != NULL) {
311+
return 0;
312+
}
313+
314+
Py_BEGIN_CRITICAL_SECTION(self);
315+
cached = self->_co_cached;
316+
if (cached == NULL) {
317+
cached = PyMem_New(_PyCoCached, 1);
318+
if (cached == NULL) {
311319
PyErr_NoMemory();
312-
return -1;
313320
}
314-
self->_co_cached->_co_code = NULL;
315-
self->_co_cached->_co_cellvars = NULL;
316-
self->_co_cached->_co_freevars = NULL;
317-
self->_co_cached->_co_varnames = NULL;
321+
else {
322+
cached->_co_code = NULL;
323+
cached->_co_cellvars = NULL;
324+
cached->_co_freevars = NULL;
325+
cached->_co_varnames = NULL;
326+
FT_ATOMIC_STORE_PTR(self->_co_cached, cached);
327+
}
318328
}
319-
return 0;
320-
329+
Py_END_CRITICAL_SECTION();
330+
return cached != NULL ? 0 : -1;
321331
}
332+
322333
/******************
323334
* _PyCode_New()
324335
******************/
@@ -1544,16 +1555,21 @@ get_cached_locals(PyCodeObject *co, PyObject **cached_field,
15441555
{
15451556
assert(cached_field != NULL);
15461557
assert(co->_co_cached != NULL);
1547-
if (*cached_field != NULL) {
1548-
return Py_NewRef(*cached_field);
1558+
PyObject *varnames = FT_ATOMIC_LOAD_PTR(*cached_field);
1559+
if (varnames != NULL) {
1560+
return Py_NewRef(varnames);
15491561
}
1550-
assert(*cached_field == NULL);
1551-
PyObject *varnames = get_localsplus_names(co, kind, num);
1562+
1563+
Py_BEGIN_CRITICAL_SECTION(co);
1564+
varnames = *cached_field;
15521565
if (varnames == NULL) {
1553-
return NULL;
1566+
varnames = get_localsplus_names(co, kind, num);
1567+
if (varnames != NULL) {
1568+
FT_ATOMIC_STORE_PTR(*cached_field, varnames);
1569+
}
15541570
}
1555-
*cached_field = Py_NewRef(varnames);
1556-
return varnames;
1571+
Py_END_CRITICAL_SECTION();
1572+
return Py_XNewRef(varnames);
15571573
}
15581574

15591575
PyObject *
@@ -1652,18 +1668,26 @@ _PyCode_GetCode(PyCodeObject *co)
16521668
if (init_co_cached(co)) {
16531669
return NULL;
16541670
}
1655-
if (co->_co_cached->_co_code != NULL) {
1656-
return Py_NewRef(co->_co_cached->_co_code);
1671+
1672+
_PyCoCached *cached = co->_co_cached;
1673+
PyObject *code = FT_ATOMIC_LOAD_PTR(cached->_co_code);
1674+
if (code != NULL) {
1675+
return Py_NewRef(code);
16571676
}
1658-
PyObject *code = PyBytes_FromStringAndSize((const char *)_PyCode_CODE(co),
1659-
_PyCode_NBYTES(co));
1677+
1678+
Py_BEGIN_CRITICAL_SECTION(co);
1679+
code = cached->_co_code;
16601680
if (code == NULL) {
1661-
return NULL;
1681+
code = PyBytes_FromStringAndSize((const char *)_PyCode_CODE(co),
1682+
_PyCode_NBYTES(co));
1683+
if (code != NULL) {
1684+
deopt_code(co, (_Py_CODEUNIT *)PyBytes_AS_STRING(code));
1685+
assert(cached->_co_code == NULL);
1686+
FT_ATOMIC_STORE_PTR(cached->_co_code, code);
1687+
}
16621688
}
1663-
deopt_code(co, (_Py_CODEUNIT *)PyBytes_AS_STRING(code));
1664-
assert(co->_co_cached->_co_code == NULL);
1665-
co->_co_cached->_co_code = Py_NewRef(code);
1666-
return code;
1689+
Py_END_CRITICAL_SECTION();
1690+
return Py_XNewRef(code);
16671691
}
16681692

16691693
PyObject *

0 commit comments

Comments
 (0)