Fix thread-safety in GC, type cache, and instruction cache (#7355) · RustPython/RustPython@375b547 · GitHub
Skip to content

Commit 375b547

Browse files
authored
Fix thread-safety in GC, type cache, and instruction cache (#7355)
* Fix thread-safety in GC, type cache, and instruction cache GC / refcount: - Add safe_inc() check for strong()==0 in RefCount - Add try_to_owned() to PyObject for atomic refcount acquire - Replace strong_count()+to_owned() with try_to_owned() in GC collection and weakref callback paths to prevent TOCTOU races Type cache: - Add proper SeqLock (sequence counter) to TypeCacheEntry - Readers spin-wait on odd sequence, validate after read - Writers bracket updates with begin_write/end_write - Use try_to_owned + pointer revalidation on read path - Call modified() BEFORE attribute modification in set_attr Instruction cache: - Add pointer_cache (AtomicUsize array) to CodeUnits for single atomic pointer load/store (prevents torn reads) - Add try_read_cached_descriptor with try_to_owned + pointer and version revalidation after increment - Add write_cached_descriptor with version-bracketed writes RLock: - Fix release() to check is_owned_by_current_thread - Add _release_save/_acquire_restore methods * Fix RLock _acquire_restore tuple handling and unxfail threading test * Align type cache seqlock writer protocol with CPython * RLock: use single parking_lot level, track recursion manually Instead of calling lock()/unlock() N times for recursion depth N, keep parking_lot at 1 level and manage the count ourselves. This makes acquire/release O(1) and matches CPython's _PyRecursiveMutex approach (lock once + set level directly). * Add try_to_owned_from_ptr to avoid &PyObject on stale ptrs Use addr_of! to access ref_count directly from a raw pointer without forming &PyObject first. Applied in type cache and instruction cache hit paths where the pointer may be stale. * Fix CI: spelling typo and xfail flaky test_thread_safety - Fix "minimising" -> "minimizing" for cspell - xfail test_thread_safety: dict iteration races with concurrent GC mutations in _finalizer_registry
1 parent 86134e1 commit 375b547

10 files changed

Lines changed: 374 additions & 181 deletions

File tree

Lib/test/_test_multiprocessing.py

Lines changed: 2 additions & 2 deletions

Lib/test/test_threading.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2141,10 +2141,6 @@ def __init__(self, a, *, b) -> None:
21412141
CustomRLock(1, b=2)
21422142
self.assertEqual(warnings_log, [])
21432143

2144-
@unittest.expectedFailure # TODO: RUSTPYTHON
2145-
def test_release_save_unacquired(self):
2146-
return super().test_release_save_unacquired()
2147-
21482144
@unittest.skip('TODO: RUSTPYTHON; flaky test')
21492145
def test_different_thread(self):
21502146
return super().test_different_thread()

crates/common/src/refcount.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ impl RefCount {
123123
pub fn safe_inc(&self) -> bool {
124124
let mut old = State::from_raw(self.state.load(Ordering::Relaxed));
125125
loop {
126-
if old.destructed() {
126+
if old.destructed() || old.strong() == 0 {
127127
return false;
128128
}
129129
if (old.strong() as usize) >= STRONG {

crates/compiler-core/src/bytecode.rs

Lines changed: 35 additions & 15 deletions

0 commit comments

Comments
 (0)