Minor Chan tweaks #11413
Closed
Minor Chan tweaks #11413
Conversation
This file contains 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
Beforehand, using a concurrent queue always mandated that the "shared state" be stored internally to the queues in order to provide a safe interface. This isn't quite as flexible as one would want in some circumstances, so instead this commit moves the queues to not containing the shared state. The queues no longer have a "default useful safe" interface, but rather a "default safe" interface (minus the useful part). The queues have to be shared manually through an Arc or some other means. This allows them to be a little more flexible at the cost of a usability hindrance. I plan on using this new flexibility to upgrade a channel to a shared channel seamlessly.
This fixes the deadlock where you send ports on channels, but then the receiving task fails, dropping the port, but the queue isn't dropped so if you then wait for data you'll wait forever.
| @@ -0,0 +1,61 @@ | |||
| // Copyright 2014 The Rust Project Developers. See the COPYRIGHT | |||
There was a problem hiding this comment.
Is there any reason this is bottled up in libgreen? I feel like this would be more generally useful as the safe layer on top of the Queue types.
There was a problem hiding this comment.
We can certainly add convenience wrappers if the need arises, but right now I don't imagine these queues will be widely used, and we should probably make them semi-private or unstable anyway.
|
If it's ok, I wanted to run this by @brson once just to make extra sure. |
|
I mentioned on IRC that I'm a little uneasy about the relaxed atomics here and what is allowed to be reordered. It looks like this code is particularly dependent on ordering. |
|
Closing, I'm lumping this into a future PR. |
flip1995
pushed a commit
to flip1995/rust
that referenced
this pull request
Sep 25, 2023
new unnecessary_map_on_constructor lint changelog: [`unnecessary_map_on_constructor`]: adds lint for cases in which map is not necessary. `Some(4).map(myfunction)` => `Some(myfunction(4))` Closes rust-lang/rust-clippy#6472 Note that the case mentioned in the issue `Some(..).and_then(|..| Some(..))` is fixed by a chain of lint changes. This PR completes the last part of that chain. By `bind_instead_of_map`[lint](https://rust-lang.github.io/rust-clippy/master/index.html#/bind_instead_of_map): `Some(4).and_then(|x| Some(foo(4)))` => `Some(4).map(|x| foo)` By `redundant_closure` [lint](https://rust-lang.github.io/rust-clippy/master/index.html#/redundant_closure): `Some(4).map(|x| foo)` => `Some(4).map(fun)` Finally by this PR `unnecessary_map_on_constructor`: `Some(4).map(fun)` => `Some(fun(4))` I'm not sure this is the desired behavior for clippy and if it should be addressed in another issue/PR. I'd be up to give it a try if that's the case.
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.
The first commit does a bit of a shuffle with how things are organized, but there are no major changes. The goal of the commit is to simplify how things are stored with respect to the concurrent queues in use (no longer producer/consumer halves).
The second commit fixes a deadlock which was uncovered in servo. I attempted to fix everything up, and I think that it's all in order now, but as always lock free code is pretty hairy so another pair of eyes would certainly be useful.
Cursory benchmarking of this change does not show any impact on message sending performance, but I have not done super in-depth benchmarking.