Ensure the coordinator thread terminates before its channels drop #145894
Conversation
|
rustbot has assigned @petrochenkov. Use |
rustbot
added
S-waiting-on-review
|
Some changes occurred in compiler/rustc_codegen_ssa |
|
r? WaffleLapkin |
bors
added
S-waiting-on-bors
| pub codegen_worker_receive: Receiver<CguMessage>, | ||
| pub shared_emitter_main: SharedEmitterMain, |
There was a problem hiding this comment.
A comment saying the field order is intentional would be helpful for future readers.
There was a problem hiding this comment.
Field order below is intended to terminate the coordinator thread before two fields below drop and prematurely close channels used by coordinator thread. See
Coordinator'sDropimplementation for more info.
There was a problem hiding this comment.
Manually implementing Drop is also common when a specific drop order is required.
There was a problem hiding this comment.
Maybe wrap intended fields in some comment block? This way new added fields to this struct didn't mix up with this description. Or maybe better impl drop for OngoingCodegen to enforce drop order, instead of comments.
There was a problem hiding this comment.
@klensy I think the current state is alright, but feel free to open a follow-up to make things nicer
There was a problem hiding this comment.
Manually implementing
Dropis also common when a specific drop order is required.
I think better place for moved fields then would be inside of Coordinator. Idk, maybe i should do that instead? That would probably be more manageable.
|
@bors r- |
bors
added
S-waiting-on-author
|
Alright, I have reproduced the issue and confirmed my hypothesis. This PR should fix it. |
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
Rollup of 5 pull requests Successful merges: - #145382 (Add assembly test for `-Zreg-struct-return` option) - #145746 (Fix STD build failing for target_os = "espidf") - #145826 (Use AcceptContext in AttribueParser::check_target) - #145894 (Ensure the coordinator thread terminates before its channels drop) - #145946 (Remove unnecessary `[dependencies.unicode-properties]` entries.) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #145894 - zetanumbers:issue-142949, r=WaffleLapkin Ensure the coordinator thread terminates before its channels drop Fixes #142949 Explanation: #142949 (comment)
Fixes #142949
Explanation: #142949 (comment)