Skip to content
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

Jump to bottom

Fix for #13163: DefaultRwLock accumulates write-waiters, eventually fails to write lock #13180

Merged
merged 5 commits into from
Oct 17, 2022
Merged

Fix for #13163: DefaultRwLock accumulates write-waiters, eventually fails to write lock #13180

merged 5 commits into from
Oct 17, 2022

Conversation

jumpnbrownweasel
Copy link
Contributor

Fixes #13163

@jumpnbrownweasel
Copy link
Contributor Author

Forgot to comment out the debug.print at the end of the last test, end of the file. I'll do that.

kprotty
kprotty reviewed Oct 16, 2022
View reviewed changes
lib/std/Thread/RwLock.zig Show resolved Hide resolved
lib/std/Thread/RwLock.zig Outdated Show resolved Hide resolved
lib/std/Thread/RwLock.zig Outdated Show resolved Hide resolved
lib/std/Thread/RwLock.zig Show resolved Hide resolved
lib/std/Thread/RwLock.zig Outdated Show resolved Hide resolved
@jumpnbrownweasel
- use equality test after lock/unlock rather than peeking into intern…
ca1e11e 
…als.

  however, this is still implementation specific and only done for
  DefaultRwLock.
- add num_reads maximum to ensure that reader threads stop if writer threads are
  starved
- use relaxed orderings for the read atomic counter
- don't check at the end for non-zero read ops, since the reader threads may
  only run once if they are starved
@jumpnbrownweasel
More review changes
8d4a5bd 
- Monotonic is sufficient for incrementing the reads counter
@jumpnbrownweasel
Copy link
Contributor Author

@kprotty I think all the review issues were resolved and all CI checks passed. Are you able to merge it, or should we ask Andrew?

@kprotty kprotty merged commit 71f8762 into ziglang:master Oct 17, 2022
@jumpnbrownweasel
Copy link
Contributor Author

@kprotty Thank you very much for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kprotty kprotty kprotty approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

DefaultRwLock accumulates write-waiters, eventually fails to write lock
2 participants
@jumpnbrownweasel @kprotty