Skip to content

Commit f9611ac

Browse files
gh-157660: Fix stale TLBC caches in _remote_debugging
`profiling.sampling` reporting errors or incorrect line numbers in free-threaded builds when a thread-local bytecode array grows or gains entries after being cached.
1 parent e2ff498 commit f9611ac

4 files changed

Lines changed: 114 additions & 4 deletions

File tree

‎Lib/test/test_external_inspection.py‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2235,6 +2235,94 @@ def get_trace_with_opcodes(pid):
22352235
location.col_offset, location.end_col_offset)
22362236
self.assertIn(actual, valid_locations)
22372237

2238+
@skip_if_not_supported
2239+
@unittest.skipIf(sys._is_gil_enabled(), "Requires free-threading")
2240+
@unittest.skipIf(
2241+
sys.platform == "linux" and not PROCESS_VM_READV_SUPPORTED,
2242+
"Requires process_vm_readv",
2243+
)
2244+
def test_tlbc_cache_refresh_after_growth(self):
2245+
# Reproducer from gh-157660.
2246+
script = textwrap.dedent("""\
2247+
import os, threading, time
2248+
from _remote_debugging import RemoteUnwinder
2249+
2250+
go = threading.Event()
2251+
stop = threading.Event()
2252+
2253+
def leaf():
2254+
stop.wait()
2255+
2256+
threading.Thread(target=leaf, daemon=True).start()
2257+
for _ in range(16):
2258+
threading.Thread(target=stop.wait, daemon=True).start()
2259+
threading.Thread(target=lambda: (go.wait(), leaf()), daemon=True).start()
2260+
2261+
time.sleep(0.1)
2262+
u = RemoteUnwinder(os.getpid(), all_threads=True, cache_frames=False)
2263+
u.get_stack_trace()
2264+
go.set()
2265+
time.sleep(0.1)
2266+
u.get_stack_trace() # RuntimeError: Invalid tlbc_index 18 (array size 16, ...)
2267+
""")
2268+
result = subprocess.run(
2269+
[sys.executable, "-X", "gil=0", "-X", "tlbc=1", "-c", script],
2270+
capture_output=True,
2271+
text=True,
2272+
timeout=SHORT_TIMEOUT,
2273+
)
2274+
self.assertEqual(
2275+
result.returncode, 0,
2276+
f"stdout: {result.stdout}\nstderr: {result.stderr}",
2277+
)
2278+
2279+
@skip_if_not_supported
2280+
@unittest.skipIf(sys._is_gil_enabled(), "Requires free-threading")
2281+
@unittest.skipIf(
2282+
sys.platform == "linux" and not PROCESS_VM_READV_SUPPORTED,
2283+
"Requires process_vm_readv",
2284+
)
2285+
def test_tlbc_cache_refresh_after_slot_fill(self):
2286+
# Reproducer from gh-157660.
2287+
script = textwrap.dedent("""\
2288+
import os, threading, time
2289+
from _remote_debugging import RemoteUnwinder
2290+
2291+
go = threading.Event()
2292+
stop = threading.Event()
2293+
2294+
def leaf():
2295+
stop.wait()
2296+
2297+
def lines(u):
2298+
return sorted(f.location.lineno for i in u.get_stack_trace()
2299+
for t in i.threads for f in t.frame_info
2300+
if f.funcname == "leaf")
2301+
2302+
threading.Thread(target=leaf, daemon=True).start()
2303+
threading.Thread(target=lambda: (go.wait(), leaf()), daemon=True).start()
2304+
time.sleep(0.1)
2305+
u = RemoteUnwinder(os.getpid(), all_threads=True, cache_frames=False)
2306+
print("Before:", lines(u))
2307+
go.set()
2308+
time.sleep(0.1)
2309+
print("Cached:", lines(u))
2310+
""")
2311+
result = subprocess.run(
2312+
[sys.executable, "-X", "gil=0", "-X", "tlbc=1", "-c", script],
2313+
capture_output=True,
2314+
text=True,
2315+
timeout=SHORT_TIMEOUT,
2316+
)
2317+
self.assertEqual(
2318+
result.returncode, 0,
2319+
f"stdout: {result.stdout}\nstderr: {result.stderr}",
2320+
)
2321+
self.assertEqual(
2322+
result.stdout,
2323+
'Before: [8]\nCached: [8, 8]\n',
2324+
)
2325+
22382326

22392327
class TestUnsupportedPlatformHandling(unittest.TestCase):
22402328
@unittest.skipIf(
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix :mod:`profiling.sampling` reporting errors or incorrect line numbers in free-threaded
2+
builds when a thread-local bytecode array grows or gains entries after being cached.

‎Modules/_remote_debugging/_remote_debugging.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,8 @@ typedef struct {
582582

583583
extern void tlbc_cache_entry_destroy(void *ptr);
584584
extern TLBCCacheEntry *get_tlbc_cache_entry(RemoteUnwinderObject *self, uintptr_t code_addr, uint32_t current_generation);
585-
extern int cache_tlbc_array(RemoteUnwinderObject *unwinder, uintptr_t code_addr, uintptr_t tlbc_array_addr, uint32_t generation);
585+
extern int cache_tlbc_array(RemoteUnwinderObject *unwinder, uintptr_t code_addr,
586+
uintptr_t tlbc_array_addr, uint32_t generation);
586587
#endif
587588

588589
/* ============================================================================

‎Modules/_remote_debugging/code_objects.c‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,12 @@ get_tlbc_cache_entry(RemoteUnwinderObject *self, uintptr_t code_addr, uint32_t c
3232
TLBCCacheEntry *entry = _Py_hashtable_get(self->tlbc_cache, key);
3333

3434
if (entry && entry->generation != current_generation) {
35-
// Entry is stale, remove it by setting to NULL
36-
_Py_hashtable_set(self->tlbc_cache, key, NULL);
37-
entry = NULL;
35+
// Entry is stale, remove it from the cache and destroy it
36+
TLBCCacheEntry *old = _Py_hashtable_steal(self->tlbc_cache, key);
37+
if (old != NULL) {
38+
tlbc_cache_entry_destroy(old);
39+
}
40+
return NULL;
3841
}
3942

4043
return entry;
@@ -449,6 +452,22 @@ parse_code_object(RemoteUnwinderObject *unwinder,
449452
tlbc_entry = get_tlbc_cache_entry(unwinder, real_address, unwinder->tlbc_generation);
450453
}
451454

455+
if (tlbc_entry && ctx->tlbc_index >= 0) {
456+
uintptr_t *entries = (uintptr_t *)((char *)tlbc_entry->tlbc_array + sizeof(Py_ssize_t));
457+
if (ctx->tlbc_index >= tlbc_entry->tlbc_array_size ||
458+
entries[ctx->tlbc_index] == 0) {
459+
TLBCCacheEntry *old = _Py_hashtable_steal(unwinder->tlbc_cache, (void *)real_address);
460+
if (old != NULL) {
461+
tlbc_cache_entry_destroy(old);
462+
}
463+
if (!cache_tlbc_array(unwinder, real_address, real_address + unwinder->debug_offsets.code_object.co_tlbc,
464+
unwinder->tlbc_generation)) {
465+
goto error;
466+
}
467+
tlbc_entry = get_tlbc_cache_entry(unwinder, real_address, unwinder->tlbc_generation);
468+
}
469+
}
470+
452471
// Validate tlbc_index and check TLBC cache
453472
if (tlbc_entry) {
454473
// Validate index bounds (also catches negative values since tlbc_index is signed)

0 commit comments

Comments
 (0)