diff --git a/src/policies/fifopolicy.rs b/src/policies/fifopolicy.rs index 4696b4d..0199c86 100644 --- a/src/policies/fifopolicy.rs +++ b/src/policies/fifopolicy.rs @@ -38,8 +38,12 @@ impl traits::OccupiedExt for Occupied<'_> { #[inline] fn replace(self, new: Self::Handle) -> Self::Handle { - // In update we don't need to increment this; because this does not change the memory address ranges - // self.shared.generation_version().increment(); + // The replace operation does not modify memory range here in the normal case, + // but when the GIL is disabled, concurrent modification can cause a segfault. + // Therefore, in free-threaded versions we have to change the generation version + // before manipulating memory. + #[cfg(Py_GIL_DISABLED)] + self.shared.generation_version().increment(); let item = unsafe { get_handle!(&mut self.policy, *self.bucket.as_ref()) }; diff --git a/src/policies/lfupolicy.rs b/src/policies/lfupolicy.rs index 5e7fc01..0742d4f 100644 --- a/src/policies/lfupolicy.rs +++ b/src/policies/lfupolicy.rs @@ -147,8 +147,12 @@ impl traits::OccupiedExt for Occupied<'_> { type Shared = Shared; fn replace(self, new: Self::Handle) -> Self::Handle { - // Here we don't need to increment generation version - // self.shared.generation_version().increment(); + // The replace operation does not modify memory range here in the normal case, + // but when the GIL is disabled, concurrent modification can cause a segfault. + // Therefore, in free-threaded versions we have to change the generation version + // before manipulating memory. + #[cfg(Py_GIL_DISABLED)] + self.shared.generation_version().increment(); unsafe { let cursor = self.bucket.as_mut(); diff --git a/src/policies/nopolicy.rs b/src/policies/nopolicy.rs index fdd4558..7fd1abe 100644 --- a/src/policies/nopolicy.rs +++ b/src/policies/nopolicy.rs @@ -33,6 +33,13 @@ impl traits::OccupiedExt for Occupied<'_> { } fn replace(self, new: Self::Handle) -> Self::Handle { + // The replace operation does not modify memory range here in the normal case, + // but when the GIL is disabled, concurrent modification can cause a segfault. + // Therefore, in free-threaded versions we have to change the generation version + // before manipulating memory. + #[cfg(Py_GIL_DISABLED)] + self.shared.generation_version().increment(); + self.policy.currsize = self.policy.currsize.saturating_add(new.size()); let old = unsafe { std::mem::replace(self.bucket.as_mut(), new) }; self.policy.currsize = self.policy.currsize.saturating_sub(old.size()); diff --git a/src/policies/rrpolicy.rs b/src/policies/rrpolicy.rs index 80b5216..b0ad113 100644 --- a/src/policies/rrpolicy.rs +++ b/src/policies/rrpolicy.rs @@ -35,6 +35,13 @@ impl traits::OccupiedExt for Occupied<'_> { #[inline] fn replace(self, new: Self::Handle) -> Self::Handle { + // The replace operation does not modify memory range here in the normal case, + // but when the GIL is disabled, concurrent modification can cause a segfault. + // Therefore, in free-threaded versions we have to change the generation version + // before manipulating memory. + #[cfg(Py_GIL_DISABLED)] + self.shared.generation_version().increment(); + self.policy.currsize = self.policy.currsize.saturating_add(new.size()); let old = unsafe { std::mem::replace(self.bucket.as_mut(), new) }; self.policy.currsize = self.policy.currsize.saturating_sub(old.size()); diff --git a/src/policies/ttlpolicy.rs b/src/policies/ttlpolicy.rs index 0c2fe6f..db44a2a 100644 --- a/src/policies/ttlpolicy.rs +++ b/src/policies/ttlpolicy.rs @@ -151,8 +151,12 @@ impl traits::OccupiedExt for Occupied<'_> { #[inline] fn replace(self, new: Self::Handle) -> Self::Handle { - // In update we don't need to increment this; because this does not change the memory address ranges - // self.shared.generation_version().increment(); + // The replace operation does not modify memory range here in the normal case, + // but when the GIL is disabled, concurrent modification can cause a segfault. + // Therefore, in free-threaded versions we have to change the generation version + // before manipulating memory. + #[cfg(Py_GIL_DISABLED)] + self.shared.generation_version().increment(); let item = unsafe { get_handle!(&mut self.policy, *self.bucket.as_ref()) }; diff --git a/src/policies/vttlpolicy.rs b/src/policies/vttlpolicy.rs index adc9a71..3b923f5 100644 --- a/src/policies/vttlpolicy.rs +++ b/src/policies/vttlpolicy.rs @@ -152,8 +152,12 @@ impl traits::OccupiedExt for Occupied<'_> { type Shared = Shared; fn replace(self, new: Self::Handle) -> Self::Handle { - // Here we don't need to increment generation version - // self.shared.generation_version().increment(); + // The replace operation does not modify memory range here in the normal case, + // but when the GIL is disabled, concurrent modification can cause a segfault. + // Therefore, in free-threaded versions we have to change the generation version + // before manipulating memory. + #[cfg(Py_GIL_DISABLED)] + self.shared.generation_version().increment(); unsafe { let cursor = self.bucket.as_mut(); diff --git a/tests/mixins.py b/tests/mixins.py index cb64dea..9fa294e 100644 --- a/tests/mixins.py +++ b/tests/mixins.py @@ -5,6 +5,7 @@ import platform import subprocess import sys +import sysconfig import threading import time import typing @@ -669,13 +670,16 @@ def test_generation_version_on_insert(self): for _ in cache.items(): cache.insert("C", 1) - if isinstance(cache, cachebox.LRUCache): - return + if not isinstance(cache, cachebox.LRUCache) and not sysconfig.get_config_var( + "Py_GIL_DISABLED" + ): + for i in cache: + cache.insert(i, "hello") - for i in cache: - # It should not increment the generation version - # in replacing value - cache.insert(i, "hello") + else: + with pytest.raises(RuntimeError): + for i in cache: + cache.insert(i, "hello") def test_generation_version_on_shrink_to_fit(self): cache = self.create_cache(10, {i: i for i in range(3)}) diff --git a/uv.lock b/uv.lock index 2e654d1..8a7ff87 100644 --- a/uv.lock +++ b/uv.lock @@ -122,9 +122,11 @@ dev = [ { name = "maturin", extra = ["patchelf"], marker = "sys_platform == 'linux'" }, { name = "mkdocs-material" }, { name = "mkdocstrings", extra = ["python"] }, + { name = "mypy" }, { name = "pytest" }, { name = "pytest-asyncio" }, { name = "pytest-xdist" }, + { name = "ruff" }, { name = "typos" }, { name = "zizmor" }, ] @@ -170,9 +172,11 @@ dev = [ { name = "maturin", extras = ["patchelf"], marker = "sys_platform == 'linux'", specifier = "==1.14.1" }, { name = "mkdocs-material", specifier = "==9.7.7" }, { name = "mkdocstrings", extras = ["python"], specifier = "==1.0.6" }, + { name = "mypy", specifier = ">=2.3.1" }, { name = "pytest", specifier = "==9.1.1" }, { name = "pytest-asyncio", specifier = "==1.4.0" }, { name = "pytest-xdist", specifier = "==3.8.0" }, + { name = "ruff", specifier = ">=0.16.9" }, { name = "typos", specifier = "==1.48.0" }, { name = "zizmor", specifier = "==1.29.0" }, ]