Call the setdefault_with factory without holding the lock - #68
Merged
Conversation
Owner
|
I hadn’t thought of something like that |
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.
Closes #67
The factory used to run while the internal lock was held, and
__traverse__takes that same lock, so a GC pass inside the factory froze the process. Now the key is looked up under the lock, the lock is dropped, the factory runs without it, and then the lock is taken again to re-check the key before inserting. Same change in all seven cache types, plus the doc strings.One behaviour change: if two threads miss the same key at the same time, the factory can run twice. The value inserted first stays in the cache and is returned to both of them, so no caller ends up holding an object the cache does not have.
The test sits in the shared mixin, so it runs for all seven types. It does the call in a child process on purpose: a deadlock keeps the GIL, so a watchdog inside the same process would never fire. Without the patch the test fails with "never returned" instead of freezing the whole run.
1004 passed, 6 skipped. The suite also stopped hanging: 4 freezes in 9 runs before, 5 clean runs out of 5 after.