Skip to content

fix: views read through their position only under the cache lock #116

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
awolverp merged 2 commits into from
Oct 28, 2026
Merged

fix: views read through their position only under the cache lock #116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d071239
test: iterate while other threads mutate the cache
greenclaw Oct 28, 2026
f2cedee
fix: views read through their position only under the cache lock
greenclaw Oct 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion docs/docs/tips.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,4 +370,9 @@ for k in it:
```

Do not modify the cache while an iterator is alive — every method on that iterator raises
`RuntimeError` if the cache changed.
`RuntimeError` if the cache changed.

Iterator methods take the cache's lock, so another thread can iterate safely while the cache
is being modified (it gets the `RuntimeError`). Don't call them from code that runs *inside* a
cache operation on the same cache, such as a key's `__eq__` or a value's `__del__`: that waits
for the lock it is already holding and never returns, the same as calling `cache[key]` there.
27 changes: 21 additions & 6 deletions src/macro_rules.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,37 @@ macro_rules! implement_pyclass {
};
}

/// Implements the generation-version guard shared by every cache view type.
/// Implements the guard shared by every cache view type.
///
/// Views hold raw positions into the policy storage. A position is valid only while
/// the cache is unchanged, and every mutation bumps the generation version while
/// holding the cache lock. So a view must take the cache lock **first**, then compare
/// the generation, and keep the lock while it reads through its position. Checking
/// without the lock races with other threads on free-threaded builds (#94).
///
/// Lock order: cache lock, then the view's own iterator mutex.
///
/// # Example
///
/// ```ignore
/// implement_view_guard!(PyCacheKeys);
/// implement_view_guard!(PyCacheKeys, nopolicy::NoPolicy);
/// ```
#[macro_export]
macro_rules! implement_view_guard {
($name:ident) => {
($name:ident, $policy:ty) => {
impl $name {
/// Fails if the cache changed after this view was created.
/// Locks the cache and fails if it changed after this view was created.
///
/// Hold the returned guard for as long as entries are read.
#[inline]
fn check_generation(&self) -> pyo3::PyResult<()> {
fn lock_and_check<'a>(
&'a self,
py: pyo3::Python<'_>,
) -> pyo3::PyResult<parking_lot::MutexGuard<'a, $policy>> {
let policy = self.cache.get().0.get()?.policy_py_attached(py);

if self.initial_gv == self.gv.get() {
return Ok(());
return Ok(policy);
}

Err($crate::new_py_error!(
Expand Down
14 changes: 7 additions & 7 deletions src/pyclasses/cache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ impl PyCache {
let initial_gv = gv.get();

let result = PyCacheItems {
cache: slf.as_any().clone().unbind(),
cache: slf.clone().unbind(),
// SAFETY: We cannot use lifetimes here, but we're tracking changes using [`GenerationVersion`]
iter: parking_lot::Mutex::new(unsafe {
inner.policy_py_attached(slf.py()).table().iter()
Expand All @@ -574,7 +574,7 @@ impl PyCache {
let initial_gv = gv.get();

let result = PyCacheValues {
cache: slf.as_any().clone().unbind(),
cache: slf.clone().unbind(),
// SAFETY: We cannot use lifetimes here, but we're tracking changes using [`GenerationVersion`]
iter: parking_lot::Mutex::new(unsafe {
inner.policy_py_attached(slf.py()).table().iter()
Expand All @@ -591,7 +591,7 @@ impl PyCache {
let initial_gv = gv.get();

let result = PyCacheKeys {
cache: slf.as_any().clone().unbind(),
cache: slf.clone().unbind(),
// SAFETY: We cannot use lifetimes here, but we're tracking changes using [`GenerationVersion`]
iter: parking_lot::Mutex::new(unsafe {
inner.policy_py_attached(slf.py()).table().iter()
Expand Down Expand Up @@ -698,14 +698,14 @@ macro_rules! implement_iterator {
$(
implement_pyclass! {
[generic, frozen] $name as $pyname {
cache: pyo3::Py<pyo3::PyAny>,
cache: pyo3::Py<PyCache>,
initial_gv: u32,
gv: utils::GenerationVersion,
iter: parking_lot::Mutex<crate::hashbrown::raw::RawIter<nopolicy::Handle>>,
}
}

implement_view_guard!($name);
implement_view_guard!($name, nopolicy::NoPolicy);

#[pyo3::pymethods]
impl $name {
Expand All @@ -719,7 +719,7 @@ macro_rules! implement_iterator {
}

fn __next__(slf: pyo3::PyRef<'_, Self>) -> pyo3::PyResult<$rt_type> {
slf.check_generation()?;
let _policy = slf.lock_and_check(slf.py())?;

let mut iter = slf.iter.lock();

Expand All @@ -735,7 +735,7 @@ macro_rules! implement_iterator {

/// Returns how many items are left to yield.
fn __len__(slf: pyo3::PyRef<'_, Self>) -> pyo3::PyResult<usize> {
slf.check_generation()?;
let _policy = slf.lock_and_check(slf.py())?;

Ok(slf.iter.lock().len())
}
Expand Down
14 changes: 7 additions & 7 deletions src/pyclasses/fifocache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ impl PyFIFOCache {

// SAFETY: We cannot use lifetimes here, but we're tracking changes using [`GenerationVersion`]
let result = PyFIFOCacheItems {
cache: slf.as_any().clone().unbind(),
cache: slf.clone().unbind(),
iter: parking_lot::Mutex::new(inner.policy_py_attached(slf.py()).iter()),
gv,
initial_gv,
Expand All @@ -574,7 +574,7 @@ impl PyFIFOCache {

// SAFETY: We cannot use lifetimes here, but we're tracking changes using [`GenerationVersion`]
let result = PyFIFOCacheValues {
cache: slf.as_any().clone().unbind(),
cache: slf.clone().unbind(),
iter: parking_lot::Mutex::new(inner.policy_py_attached(slf.py()).iter()),
gv,
initial_gv,
Expand All @@ -589,7 +589,7 @@ impl PyFIFOCache {

// SAFETY: We cannot use lifetimes here, but we're tracking changes using [`GenerationVersion`]
let result = PyFIFOCacheKeys {
cache: slf.as_any().clone().unbind(),
cache: slf.clone().unbind(),
iter: parking_lot::Mutex::new(inner.policy_py_attached(slf.py()).iter()),
gv,
initial_gv,
Expand Down Expand Up @@ -716,14 +716,14 @@ macro_rules! implement_iterator {
$(
implement_pyclass! {
[generic, frozen] $name as $pyname {
cache: pyo3::Py<pyo3::PyAny>,
cache: pyo3::Py<PyFIFOCache>,
initial_gv: u32,
gv: utils::GenerationVersion,
iter: parking_lot::Mutex<utils::RawVecDequeIter<fifopolicy::Handle>>,
}
}

implement_view_guard!($name);
implement_view_guard!($name, fifopolicy::FIFOPolicy);

#[pyo3::pymethods]
impl $name {
Expand All @@ -737,7 +737,7 @@ macro_rules! implement_iterator {
}

fn __next__(slf: pyo3::PyRef<'_, Self>) -> pyo3::PyResult<$rt_type> {
slf.check_generation()?;
let _policy = slf.lock_and_check(slf.py())?;

let mut iter = slf.iter.lock();

Expand All @@ -753,7 +753,7 @@ macro_rules! implement_iterator {

/// Returns how many items are left to yield.
fn __len__(slf: pyo3::PyRef<'_, Self>) -> pyo3::PyResult<usize> {
slf.check_generation()?;
let _policy = slf.lock_and_check(slf.py())?;

Ok(slf.iter.lock().len())
}
Expand Down
16 changes: 8 additions & 8 deletions src/pyclasses/lfucache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ impl PyLFUCache {
let iter = policy.iter(gv);

let result = PyLFUCacheItems {
cache: slf.as_any().clone().unbind(),
cache: slf.clone().unbind(),
iter: parking_lot::Mutex::new(iter),
gv: gv.clone(),
initial_gv: gv.get(),
Expand All @@ -595,7 +595,7 @@ impl PyLFUCache {
let iter = policy.iter(gv);

let result = PyLFUCacheValues {
cache: slf.as_any().clone().unbind(),
cache: slf.clone().unbind(),
iter: parking_lot::Mutex::new(iter),
gv: gv.clone(),
initial_gv: gv.get(),
Expand All @@ -611,7 +611,7 @@ impl PyLFUCache {
let iter = policy.iter(gv);

let result = PyLFUCacheKeys {
cache: slf.as_any().clone().unbind(),
cache: slf.clone().unbind(),
iter: parking_lot::Mutex::new(iter),
gv: gv.clone(),
initial_gv: gv.get(),
Expand All @@ -634,7 +634,7 @@ impl PyLFUCache {
let iter = policy.iter(gv);

let result = PyLFUCacheItemsWithFrequency {
cache: slf.as_any().clone().unbind(),
cache: slf.clone().unbind(),
iter: parking_lot::Mutex::new(iter),
gv: gv.clone(),
initial_gv: gv.get(),
Expand Down Expand Up @@ -783,14 +783,14 @@ macro_rules! implement_iterator {
$(
implement_pyclass! {
[generic, frozen] $name as $pyname {
cache: pyo3::Py<pyo3::PyAny>,
cache: pyo3::Py<PyLFUCache>,
initial_gv: u32,
gv: utils::GenerationVersion,
iter: parking_lot::Mutex<lazyheap::RawIter<lfupolicy::FrequencyHandle>>,
}
}

implement_view_guard!($name);
implement_view_guard!($name, lfupolicy::LFUPolicy);

#[pyo3::pymethods]
impl $name {
Expand All @@ -804,7 +804,7 @@ macro_rules! implement_iterator {
}

fn __next__(slf: pyo3::PyRef<'_, Self>) -> pyo3::PyResult<$rt_type> {
slf.check_generation()?;
let _policy = slf.lock_and_check(slf.py())?;

let mut iter = slf.iter.lock();

Expand All @@ -820,7 +820,7 @@ macro_rules! implement_iterator {

/// Returns how many items are left to yield.
fn __len__(slf: pyo3::PyRef<'_, Self>) -> pyo3::PyResult<usize> {
slf.check_generation()?;
let _policy = slf.lock_and_check(slf.py())?;

Ok(slf.iter.lock().len())
}
Expand Down
14 changes: 7 additions & 7 deletions src/pyclasses/lrucache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ impl PyLRUCache {

// SAFETY: We cannot use lifetimes here, but we're tracking changes using [`GenerationVersion`]
let result = PyLRUCacheItems {
cache: slf.as_any().clone().unbind(),
cache: slf.clone().unbind(),
iter: parking_lot::Mutex::new(unsafe {
inner.policy_py_attached(slf.py()).list().iter()
}),
Expand All @@ -603,7 +603,7 @@ impl PyLRUCache {

// SAFETY: We cannot use lifetimes here, but we're tracking changes using [`GenerationVersion`]
let result = PyLRUCacheValues {
cache: slf.as_any().clone().unbind(),
cache: slf.clone().unbind(),
iter: parking_lot::Mutex::new(unsafe {
inner.policy_py_attached(slf.py()).list().iter()
}),
Expand All @@ -620,7 +620,7 @@ impl PyLRUCache {

// SAFETY: We cannot use lifetimes here, but we're tracking changes using [`GenerationVersion`]
let result = PyLRUCacheKeys {
cache: slf.as_any().clone().unbind(),
cache: slf.clone().unbind(),
iter: parking_lot::Mutex::new(unsafe {
inner.policy_py_attached(slf.py()).list().iter()
}),
Expand Down Expand Up @@ -770,14 +770,14 @@ macro_rules! implement_iterator {
$(
implement_pyclass! {
[generic, frozen] $name as $pyname {
cache: pyo3::Py<pyo3::PyAny>,
cache: pyo3::Py<PyLRUCache>,
initial_gv: u32,
gv: utils::GenerationVersion,
iter: parking_lot::Mutex<linked_list::RawIter<lrupolicy::Handle>>,
}
}

implement_view_guard!($name);
implement_view_guard!($name, lrupolicy::LRUPolicy);

#[pyo3::pymethods]
impl $name {
Expand All @@ -791,7 +791,7 @@ macro_rules! implement_iterator {
}

fn __next__(slf: pyo3::PyRef<'_, Self>) -> pyo3::PyResult<$rt_type> {
slf.check_generation()?;
let _policy = slf.lock_and_check(slf.py())?;

let mut iter = slf.iter.lock();

Expand All @@ -807,7 +807,7 @@ macro_rules! implement_iterator {

/// Returns how many items are left to yield.
fn __len__(slf: pyo3::PyRef<'_, Self>) -> pyo3::PyResult<usize> {
slf.check_generation()?;
let _policy = slf.lock_and_check(slf.py())?;

Ok(slf.iter.lock().len())
}
Expand Down
14 changes: 7 additions & 7 deletions src/pyclasses/rrcache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ impl PyRRCache {

// SAFETY: We cannot use lifetimes here, but we're tracking changes using [`GenerationVersion`]
let result = PyRRCacheItems {
cache: slf.as_any().clone().unbind(),
cache: slf.clone().unbind(),
iter: parking_lot::Mutex::new(unsafe {
inner.policy_py_attached(slf.py()).table().iter()
}),
Expand All @@ -576,7 +576,7 @@ impl PyRRCache {

// SAFETY: We cannot use lifetimes here, but we're tracking changes using [`GenerationVersion`]
let result = PyRRCacheValues {
cache: slf.as_any().clone().unbind(),
cache: slf.clone().unbind(),
iter: parking_lot::Mutex::new(unsafe {
inner.policy_py_attached(slf.py()).table().iter()
}),
Expand All @@ -593,7 +593,7 @@ impl PyRRCache {

// SAFETY: We cannot use lifetimes here, but we're tracking changes using [`GenerationVersion`]
let result = PyRRCacheKeys {
cache: slf.as_any().clone().unbind(),
cache: slf.clone().unbind(),
iter: parking_lot::Mutex::new(unsafe {
inner.policy_py_attached(slf.py()).table().iter()
}),
Expand Down Expand Up @@ -712,14 +712,14 @@ macro_rules! implement_iterator {
$(
implement_pyclass! {
[generic, frozen] $name as $pyname {
cache: pyo3::Py<pyo3::PyAny>,
cache: pyo3::Py<PyRRCache>,
initial_gv: u32,
gv: utils::GenerationVersion,
iter: parking_lot::Mutex<crate::hashbrown::raw::RawIter<rrpolicy::Handle>>,
}
}

implement_view_guard!($name);
implement_view_guard!($name, rrpolicy::RRPolicy);

#[pyo3::pymethods]
impl $name {
Expand All @@ -733,7 +733,7 @@ macro_rules! implement_iterator {
}

fn __next__(slf: pyo3::PyRef<'_, Self>) -> pyo3::PyResult<$rt_type> {
slf.check_generation()?;
let _policy = slf.lock_and_check(slf.py())?;

let mut iter = slf.iter.lock();

Expand All @@ -749,7 +749,7 @@ macro_rules! implement_iterator {

/// Returns how many items are left to yield.
fn __len__(slf: pyo3::PyRef<'_, Self>) -> pyo3::PyResult<usize> {
slf.check_generation()?;
let _policy = slf.lock_and_check(slf.py())?;

Ok(slf.iter.lock().len())
}
Expand Down
Loading
Loading