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

Drop all messages in bounded channel when destroying the last receiver #108164

Merged
merged 4 commits into from
Mar 21, 2023
Merged

Drop all messages in bounded channel when destroying the last receiver #108164

merged 4 commits into from
Mar 21, 2023

Conversation

joboet
Copy link
Contributor

@joboet joboet commented Feb 17, 2023

Fixes #107466 by splitting the disconnect function for receivers/transmitters and dropping all messages in disconnect_receivers like the unbounded channel does. Since all receivers must be dropped before the channel is, the messages will already be discarded at that point, so the Drop implementation for the channel can be removed.

@rustbot label +T-libs +A-concurrency

@joboet
std: drop all messages in bounded channel when destroying the last re…
746331e 
…ceiver
@joboet
std: add regression test for rust-lang#107466
642a324 
Tests that messages are immediately dropped once the last receiver is destroyed.
@rustbot
Copy link
Collaborator

rustbot commented Feb 17, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 17, 2023
@rustbot
Copy link
Collaborator

rustbot commented Feb 17, 2023

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rustbot rustbot added the A-concurrency Area: Concurrency related issues. label Feb 17, 2023
@Mark-Simulacrum
Copy link
Member

r? @Amanieu

Should this also be replicated in a PR to crossbeam?

@rustbot rustbot assigned Amanieu and unassigned Mark-Simulacrum Feb 25, 2023
@ryoqun
Copy link
Contributor

ryoqun commented Feb 28, 2023

Should this also be replicated in a PR to crossbeam?

That'll be much appreciated because my pr (crossbeam-rs/crossbeam#959) at crossbeam will be based on this when extending my change to the Array flavor as well. Currently, only List one is supported...

@Amanieu
Copy link
Member

Amanieu commented Mar 8, 2023

LGTM but I'd like a second opinion from @ibraheemdev or @taiki-e.

@taiki-e taiki-e mentioned this pull request Mar 11, 2023
Open
bors bot added a commit to crossbeam-rs/crossbeam that referenced this pull request Mar 11, 2023
@bors @taiki-e
Merge #968
599e975 
968: channel: Extend panic_on_drop test r=taiki-e a=taiki-e

Include rust-lang/rust#107466 's case.
cc rust-lang/rust#108164


Co-authored-by: Taiki Endo <te316e89@gmail.com>
@Amanieu
Copy link
Member

Amanieu commented Mar 19, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Mar 19, 2023

📌 Commit 34aa872 has been approved by Amanieu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 19, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 20, 2023
@matthiaskrgr
Rollup merge of rust-lang#108164 - joboet:discard_messages_mpmc_array…
7c961a3 
…, r=Amanieu

Drop all messages in bounded channel when destroying the last receiver

Fixes rust-lang#107466 by splitting the `disconnect` function for receivers/transmitters and dropping all messages in `disconnect_receivers` like the unbounded channel does. Since all receivers must be dropped before the channel is, the messages will already be discarded at that point, so the `Drop` implementation for the channel can be removed.

`@rustbot` label +T-libs +A-concurrency
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 20, 2023
Closed
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 21, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 21, 2023
@bors
Auto merge of rust-lang#109453 - matthiaskrgr:rollup-odn02wu, r=matth…
1db9c06 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#96391 (Windows: make `Command` prefer non-verbatim paths)
 - rust-lang#108164 (Drop all messages in bounded channel when destroying the last receiver)
 - rust-lang#108729 (fix: modify the condition that `resolve_imports` stops)
 - rust-lang#109336 (Constrain const vars to error if const types are mismatched)
 - rust-lang#109403 (Avoid ICE of attempt to add with overflow in emitter)
 - rust-lang#109415 (Refactor `handle_missing_lit`.)
 - rust-lang#109441 (Only implement Fn* traits for extern "Rust" safe function pointers and items)
 - rust-lang#109446 (Do not suggest bounds restrictions for synthesized RPITITs)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 93a82a4 into rust-lang:master Mar 21, 2023
@rustbot rustbot added this to the 1.70.0 milestone Mar 21, 2023
@joboet joboet deleted the discard_messages_mpmc_array branch March 22, 2023 08:20
@ryoqun ryoqun mentioned this pull request Apr 19, 2023
Closed
@ryoqun
Copy link
Contributor

ryoqun commented May 27, 2023

Should this also be replicated in a PR to crossbeam?

That'll be much appreciated because my pr (crossbeam-rs/crossbeam#959) at crossbeam will be based on this when extending my change to the Array flavor as well. Currently, only List one is supported...

(I wonder when this patch is upstreamed to the crossbeam repo...? is there some place for me to check the progress? context: i just want my crossbeam-rs/crossbeam#959 to move forward)

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

@taiki-e taiki-e taiki-e approved these changes

Assignees

@Amanieu Amanieu

Labels
A-concurrency Area: Concurrency related issues. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.70.0
Development

Successfully merging this pull request may close these issues.

MPSC Sender channel not dropping buffer when all Receivers are dropped during a thread panic unwind
7 participants
@joboet @rustbot @Mark-Simulacrum @ryoqun @Amanieu @bors @taiki-e