Skip to content

Commit 4245b13

Browse files
author
Pablo Galindo Salgado
committed
fixup! fixup! small cleanp
1 parent 05799a7 commit 4245b13

2 files changed

Lines changed: 23 additions & 15 deletions

File tree

Modules/_remote_debugging/_remote_debugging.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -239,11 +239,17 @@ typedef struct {
239239
# error "INTERPRETER_THREAD_CACHE_SIZE must be a power of two"
240240
#endif
241241

242+
// The two per-interpreter L2 caches below are split into per-field tables so
243+
// that a writer rebinding one slot cannot leave stale data in a field owned by
244+
// the other when the slot is reused across interpreters.
242245
typedef struct {
243246
uintptr_t interpreter_addr;
244247
uintptr_t thread_state_addr;
248+
} InterpreterTstateCacheEntry;
249+
typedef struct {
250+
uintptr_t interpreter_addr;
245251
uint64_t code_object_generation;
246-
} InterpreterThreadCacheEntry;
252+
} InterpreterGenerationCacheEntry;
247253

248254
// Carries already-read thread state and/or frame buffers across helpers so the
249255
// downstream callee can skip a remote read. Address fields are caller-supplied
@@ -367,7 +373,8 @@ typedef struct {
367373
RemoteDebuggingState *cached_state;
368374
FrameCacheEntry *frame_cache; // preallocated array of FRAME_CACHE_MAX_THREADS entries
369375
UnwinderStats stats; // statistics for performance analysis
370-
InterpreterThreadCacheEntry cached_tstates[INTERPRETER_THREAD_CACHE_SIZE];
376+
InterpreterTstateCacheEntry cached_tstates[INTERPRETER_THREAD_CACHE_SIZE];
377+
InterpreterGenerationCacheEntry cached_generations[INTERPRETER_THREAD_CACHE_SIZE];
371378
#ifdef Py_GIL_DISABLED
372379
uint32_t tlbc_generation;
373380
_Py_hashtable_t *tlbc_cache;

Modules/_remote_debugging/module.c

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,7 @@ _remote_debugging_RemoteUnwinder___init___impl(RemoteUnwinderObject *self,
364364
self->cached_tstate_interpreter_addr = 0;
365365
self->cached_tstate_addr = 0;
366366
memset(self->cached_tstates, 0, sizeof(self->cached_tstates));
367+
memset(self->cached_generations, 0, sizeof(self->cached_generations));
367368
self->debug = debug;
368369
self->only_active_thread = only_active_thread;
369370
self->mode = mode;
@@ -482,7 +483,7 @@ interpreter_thread_cache_index(uintptr_t interpreter_addr)
482483
{
483484
// Direct-mapped table indexed by the remote interpreter address. Each entry
484485
// stores the full address and verifies it on lookup, so hash collisions
485-
// degrade to misses and cannot return a wrong tstate.
486+
// degrade to misses and cannot return a value from the wrong interpreter.
486487
return (size_t)_Py_HashPointerRaw((const void *)interpreter_addr)
487488
& (INTERPRETER_THREAD_CACHE_SIZE - 1);
488489
}
@@ -500,7 +501,7 @@ get_cached_tstate_for_interpreter(
500501
return self->cached_tstate_addr;
501502
}
502503

503-
InterpreterThreadCacheEntry *entry =
504+
InterpreterTstateCacheEntry *entry =
504505
&self->cached_tstates[interpreter_thread_cache_index(interpreter_addr)];
505506
if (entry->interpreter_addr == interpreter_addr) {
506507
self->cached_tstate_interpreter_addr = interpreter_addr;
@@ -523,7 +524,7 @@ set_cached_tstate_for_interpreter(
523524
self->cached_tstate_interpreter_addr = interpreter_addr;
524525
self->cached_tstate_addr = thread_state_addr;
525526

526-
InterpreterThreadCacheEntry *entry =
527+
InterpreterTstateCacheEntry *entry =
527528
&self->cached_tstates[interpreter_thread_cache_index(interpreter_addr)];
528529
entry->interpreter_addr = interpreter_addr;
529530
entry->thread_state_addr = thread_state_addr;
@@ -545,17 +546,17 @@ refresh_generation_caches_from_interp_state(
545546
}
546547
}
547548
else {
548-
InterpreterThreadCacheEntry *entry =
549-
&self->cached_tstates[interpreter_thread_cache_index(interpreter_addr)];
550-
uint64_t prev_generation = 0;
551-
if (entry->interpreter_addr == interpreter_addr) {
552-
prev_generation = entry->code_object_generation;
553-
}
554-
else {
555-
entry->interpreter_addr = interpreter_addr;
556-
}
549+
InterpreterGenerationCacheEntry *entry =
550+
&self->cached_generations[interpreter_thread_cache_index(interpreter_addr)];
551+
// A slot rebound from another interpreter must be treated as changed:
552+
// the code_object_cache is global, so even if the new generation
553+
// numerically matches what the previous occupant had, stale entries
554+
// from that occupant could still be served.
555+
int changed = entry->interpreter_addr != interpreter_addr
556+
|| entry->code_object_generation != code_object_generation;
557+
entry->interpreter_addr = interpreter_addr;
557558
entry->code_object_generation = code_object_generation;
558-
if (code_object_generation != prev_generation) {
559+
if (changed) {
559560
_Py_hashtable_clear(self->code_object_cache);
560561
}
561562
self->cached_generation_interpreter_addr = interpreter_addr;

0 commit comments

Comments
 (0)