Repository navigation
fix: wait for the cache lock detached in every method - #117
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #95. That fix made the insert/get/remove paths wait for the cache lock with
lock_py_attached, but the other methods still callWrapped::policy(), a plainlock()that blocks while attached to the interpreter:__len__,__bool__,__sizeof__,capacity,current_size,remaining_size,is_empty,is_full,popitem,shrink_to_fit,clear,__clear__, plusexpire/popitem_with_expireon the TTL caches. They deadlock the same way #95 did, and the GIL build is affected too:On 3.13 this never prints
ok:clear()holds the lock while the values are dropped, a__del__hands the GIL to the reader, and the reader blocks inlen()holding the GIL. On free-threaded builds a thread blocked in a plainlock()holds up a stop-the-world (thread start, GC) that the lock holder is waiting for.Change
inner.policy()insrc/pyclasses/becomesinner.policy_py_attached(py); the methods that had nopyargument get one (PyO3 injects it, the Python signatures and.pyido not change).Wrapped::remaining_sizetakespyand waits the same way.Wrapped::policy()is removed, so a new plain-lock call site no longer compiles; the doc comment oninnersays why.No behaviour change when the lock is free:
lock_py_attachedtriestry_lock()first and only detaches on contention. A single-thread micro-benchmark of the changed methods (len,bool,is_full,capacity,current_size,popitem+ set; LRU and TTL, release builds, min of 5 interleaved runs) shows differences within ±1 ns in both directions betweenmainand this branch.Test
test_lock_waiters_with_finalizersruns in a child process liketest_concurrent_inserts_with_finalizers: batches of 8 short-lived threads insert values with a__del__and call the methods above at random.main(run): it hangs for all 7 classes on 3.10–3.15 (ubuntu, macos, windows), for 6 of 7 on 3.14t and 3.15t; pypy3.11 does not reproduce.main: 21/21 runs hang on 3.13, 29/35 on 3.14t; this branch: 70/70 and 70/70 pass.Not in this PR:
__eq__/__ne__lock the two caches in argument order, soa == bandb == aon two threads can still deadlock. I can open a separate issue for that.