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.

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

Give deadlock handler access to GlobalCtxt #121971

Closed
wants to merge 2 commits into from

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Mar 4, 2024

Doesn't work yet, because we try to share a Compiler across thread boundaries, which mostly doesn't work because some things like FreezeLock are not Sync, but only DynSync.

intended as a replacement for #115220

cc @Zoxc do you think this is doable once we nuke the non-parallel-compiler cfgs and do some cleanups by using the thread safe sync primitives everywhere?

@rustbot
Copy link
Collaborator

rustbot commented Mar 4, 2024

r? @estebank

rustbot has assigned @estebank.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 4, 2024
@rust-log-analyzer

This comment has been minimized.

@oli-obk oli-obk force-pushed the lrc_gcx branch 2 times, most recently from 1f3976a to bcaca33 Compare March 5, 2024 05:26
@Zoxc
Copy link
Contributor

Zoxc commented Mar 6, 2024

You might need to opt Compiler out of Sync and DynSync to prevent concurrent enter calls.

@oli-obk oli-obk marked this pull request as ready for review March 7, 2024 10:57
@@ -311,6 +318,12 @@ impl Compiler {
{
// Must declare `_timer` first so that it is dropped after `queries`.
let mut _timer = None;
let _guard = rustc_data_structures::defer(|| {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make more sense for this to be declared after queries so that we store null before the destructor of GlobalCtxt gains unique access, even though it offers no synchronization.

@bors
Copy link
Contributor

bors commented Mar 9, 2024

☔ The latest upstream changes (presumably #122241) made this pull request unmergeable. Please resolve the merge conflicts.

@oli-obk oli-obk closed this Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants