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

Invalid usage of &mut self in std::sync::mpsc #36934

Closed
alexcrichton opened this issue Oct 3, 2016 · 2 comments
Closed

Invalid usage of &mut self in std::sync::mpsc #36934

alexcrichton opened this issue Oct 3, 2016 · 2 comments
Assignees
@apasel422
Labels
E-help-wanted Call for participation: Help is requested to fix this issue. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness

Comments

@alexcrichton
Copy link
Member

Almost all of the internal code in the mpsc module makes use of &mut self, which isn't valid because it's all explicitly designed to work across multiple threads, meaning the guarantees of &mut self don't hold. This entire implementation dates back to Fall 2013, which is long before the multithreading of &self vs &mut self was even decided!

We should update these implementation details to basically never use &mut self but instead use UnsafeCell where appropriate.
@alexcrichton alexcrichton mentioned this issue Oct 3, 2016
Merged
@apasel422 apasel422 added the I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness label Nov 19, 2016
@apasel422 apasel422 added the E-help-wanted Call for participation: Help is requested to fix this issue. label Dec 5, 2016
@apasel422 apasel422 self-assigned this Dec 16, 2016
@apasel422 apasel422 mentioned this issue Dec 17, 2016
Merged
sanxiyn added a commit to sanxiyn/rust that referenced this issue Dec 19, 2016
@sanxiyn
Rollup merge of rust-lang#38421 - apasel422:issue-36934, r=alexcrichton
4b5cffc 
Replace invalid use of `&mut` with `UnsafeCell` in `std::sync::mpsc`

Closes rust-lang#36934

r? @alexcrichton
@jnicholls
Copy link

Could this be a root cause for why this unreachable is being hit, due to a spurious wake up? https://github.com/rust-lang/rust/blob/1.12.0/src/libstd/sync/mpsc/mod.rs#L880 Seeing Empty condition in 1.12.0.

@alexcrichton
Copy link
Member Author

@jnicholls perhaps yeah, but AFAIK #38421 didn't have any substantive change beyond a standard refactor, so my guess is that it'd be unlikely

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

@apasel422 apasel422

Labels
E-help-wanted Call for participation: Help is requested to fix this issue. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jnicholls @alexcrichton @apasel422