Skip to content

Commit 8de87a5

Browse files
committed
gh-159136: Keep eval-frame recorded function names alive
Hold a strong reference to func_name in both recording hooks before PyList_Append can detach while acquiring the recording list lock.
1 parent fe6a3d5 commit 8de87a5

5 files changed

Lines changed: 108 additions & 4 deletions

File tree

‎Lib/test/test_capi/test_misc.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2914,6 +2914,24 @@ def test_is_specialization_enabled_with_allow(self):
29142914
finally:
29152915
_testinternalcapi.set_eval_frame_default()
29162916

2917+
def test_record_replaced_function_name(self):
2918+
def target(arg):
2919+
pass
2920+
2921+
records = []
2922+
names = [f'dynamic_name_{i}' for i in range(100)]
2923+
try:
2924+
_testinternalcapi.set_eval_frame_interp(records)
2925+
for name in names:
2926+
target.__name__ = name
2927+
# Call through C to avoid inlining bypassing the hook.
2928+
list(map(target, [None]))
2929+
finally:
2930+
_testinternalcapi.set_eval_frame_default()
2931+
self.assertEqual(
2932+
[name for name in records if name.startswith('dynamic_name_')],
2933+
names)
2934+
29172935
def test_allow_specialization_call(self):
29182936
def func():
29192937
pass

‎Lib/test/test_free_threading/test_functions.py‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,39 @@ def reader():
168168
threading_helper.run_concurrently(
169169
[writer, reader] * (NUM_THREADS // 2))
170170

171+
def test_eval_frame_record_while_replacing_name(self):
172+
from test.support import import_helper
173+
_testinternalcapi = import_helper.import_module('_testinternalcapi')
174+
175+
def target():
176+
pass
177+
178+
records = []
179+
180+
def writer():
181+
for i in range(ITERS):
182+
target.__name__ = f'target_{i}'
183+
184+
def caller():
185+
for _ in range(ITERS):
186+
target()
187+
188+
def contend():
189+
for _ in range(ITERS):
190+
# Contend with the recording hook for the list's lock.
191+
records.extend(['padding'] * 1000)
192+
records.clear()
193+
194+
try:
195+
_testinternalcapi.set_eval_frame_record(records)
196+
threading_helper.run_concurrently(
197+
[writer, caller, contend, caller])
198+
target()
199+
finally:
200+
_testinternalcapi.set_eval_frame_default()
201+
self.assertTrue(any(name.startswith('target') for name in records))
202+
self.assertTrue(all(isinstance(name, str) for name in records))
203+
171204
def test_argument_errors_while_replacing_qualname(self):
172205
def target(x, *, y):
173206
pass

‎Lib/test/test_funcattrs.py‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,27 @@ def __eq__(self, other):
8181
gc.collect()
8282
self.assertIsNone(ref())
8383

84+
def test_annotations_setter_reentrant_finalizer(self):
85+
def target():
86+
pass
87+
88+
observations = []
89+
replacement = {'replacement': int}
90+
finalizer_value = {'finalizer': str}
91+
92+
class Annotations(dict):
93+
def __del__(self):
94+
observations.append((target.__annotate__,
95+
target.__annotations__ is replacement))
96+
target.__annotations__ = finalizer_value
97+
98+
target.__annotate__ = lambda format: Annotations()
99+
self.assertIsInstance(target.__annotations__, Annotations)
100+
target.__annotations__ = replacement
101+
support.gc_collect()
102+
self.assertEqual(observations, [(None, True)])
103+
self.assertIs(target.__annotations__, finalizer_value)
104+
84105
def test_dir_includes_correct_attrs(self):
85106
self.b.known_attr = 7
86107
self.assertIn('known_attr', dir(self.b),

‎Lib/test/test_lltrace.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,36 @@ def test_lltrace_different_module(self):
9191
self.assertEqual(instr_map[offset].opname, opname)
9292
self.assertEqual(instr_map[offset].arg, oparg)
9393

94+
def test_lltrace_replacing_function_attributes(self):
95+
stdout = self.run_code("""
96+
import gc
97+
98+
def target():
99+
pass
100+
101+
class Name(str):
102+
def __repr__(self):
103+
target.__qualname__ = 'replacement'
104+
gc.collect()
105+
return "'original_name'"
106+
107+
class Module:
108+
def __repr__(self):
109+
target.__module__ = None
110+
gc.collect()
111+
return "'original_module'"
112+
113+
target.__qualname__ = Name('original_name')
114+
target.__module__ = Module()
115+
__lltrace__ = 1
116+
target()
117+
del __lltrace__
118+
assert target.__qualname__ == 'replacement'
119+
assert target.__module__ is None
120+
""")
121+
self.assertIn("'original_name'", stdout)
122+
self.assertIn("'original_module'", stdout)
123+
94124
def test_lltrace_does_not_crash_on_subscript_operator(self):
95125
# If this test fails, it will reproduce a crash reported as
96126
# bpo-34113. The crash happened at the command line console of

‎Modules/_testinternalcapi.c‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1142,8 +1142,9 @@ record_eval(PyThreadState *tstate, struct _PyInterpreterFrame *f, int exc)
11421142
assert(module != NULL);
11431143
module_state *state = get_module_state(module);
11441144
Py_DECREF(module);
1145-
int res = PyList_Append(state->record_list,
1146-
FT_ATOMIC_LOAD_PTR_ACQUIRE(func->func_name));
1145+
PyObject *name = Py_NewRef(FT_ATOMIC_LOAD_PTR_ACQUIRE(func->func_name));
1146+
int res = PyList_Append(state->record_list, name);
1147+
Py_DECREF(name);
11471148
if (res < 0) {
11481149
return NULL;
11491150
}
@@ -1204,8 +1205,9 @@ record_eval_interp(PyThreadState *tstate, struct _PyInterpreterFrame *f, int exc
12041205
assert(module != NULL);
12051206
module_state *state = get_module_state(module);
12061207
Py_DECREF(module);
1207-
int res = PyList_Append(state->record_list,
1208-
FT_ATOMIC_LOAD_PTR_ACQUIRE(func->func_name));
1208+
PyObject *name = Py_NewRef(FT_ATOMIC_LOAD_PTR_ACQUIRE(func->func_name));
1209+
int res = PyList_Append(state->record_list, name);
1210+
Py_DECREF(name);
12091211
if (res < 0) {
12101212
return NULL;
12111213
}

0 commit comments

Comments
 (0)