Use per-object locking instead of global locking#452
Merged
BoboTiG merged 4 commits intoBoboTiG:mainfrom Jan 5, 2026
Merged
Conversation
The global lock just provides too much of a surface for contention and deadlocks. Global locking is still needed by the Xlib backend, since Xlib is not thread-safe and we're not enabling the parts that make it slightly more thread-safe, so it now has its own lock object. The docs were also updated to spell out our multithreading guarantees.
Owner
|
Thank you @jholveck 🥂 That looks good for a first check on my end 👍 (Think about the changelog entry ;)) |
Contributor
Author
|
Sorry I keep forgetting to include the CHANGELOG entry. Thanks for keeping everything clean and documented! |
I asked GitHub Copilot to review my changes. It didn't see any problems, but I looked at its thought process and cleaned up things that it had to think carefully about. * Fix typo in an older CHANGELOG entry. * Add comments about why each documented public attribute of MSSBase, and a few undocumented but not _-prefixed attributes of MSS implementations, are safe to not be protected under the lock. * Document that the user isn't supposed to change with_cursor. Also document that MSS might ignore it. * Clarify that the comment "The attributes below are protected by self._lock" is meant to contrast with the attributes above that comment. * In MSSBase.monitors, read the return value self._monitors under the lock. Current code doesn't ever change self._monitors once it's created, so it's safe to return outside the lock, but verifying that requires inspection of the rest of the codebase. This makes it clear and future-proof. * Add a test that we can use the same MSS object on multiple threads.
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
2 participants
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.
The global lock just provides too much of a surface for contention and deadlocks. Global locking is still needed by the Xlib backend, since Xlib is not thread-safe and we're not enabling the parts that make it slightly more thread-safe, so it now has its own lock object.
The docs were also updated to spell out our multithreading guarantees.
I'm marking this PR as a draft because I want to review it more closely, rather than submitting a quick bit of weekend code.
Changes proposed in this PR
It is very important to keep up to date tests and documentation.
Is your code right?
./check.shpassed