Skip to content

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Jun 6, 2023

This moves the call to make_query outside the parts that holds the active queries lock in try_collect_active_jobs. This should help removed the deadlock and borrow panic that has been observed when printing the query stack during an ICE.

cc @SparrowLii
r? @cjgillot

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) 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 Jun 6, 2023
@SparrowLii
Copy link
Member

SparrowLii commented Jun 6, 2023

I tried your change locally (making the compiler fail with #![deny(missing_debug_implementations)] when compiling the rayon crate), but the deadlock persists : (

However, I think this PR can be merged, since it cuts some of the possible causes of deadlocks.

@SparrowLii
Copy link
Member

SparrowLii commented Jun 6, 2023

And I tried to execute the generic implementation of def_span in make_query, which does avoid deadlocks.

@cjgillot
Copy link
Contributor

cjgillot commented Jun 7, 2023

This makes sense.
@bors r+

@bors
Copy link
Collaborator

bors commented Jun 7, 2023

📌 Commit fd3d2d4 has been approved by cjgillot

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 Jun 7, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 8, 2023
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#109953 (Use 128 bits for TypeId hash)
 - rust-lang#112333 (Don't hold the active queries lock while calling `make_query`)
 - rust-lang#112339 (Fix rust-analyzer proc macro server)
 - rust-lang#112410 (Do `fix_*_builtin_expr` hacks on the writeback results)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4f2e1df into rust-lang:master Jun 8, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jun 8, 2023
@Zoxc Zoxc deleted the try_collect_active_jobs-deadlock branch June 9, 2023 03:08
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 22, 2023
…ce, r=cjgillot

Revert "Don't hold the active queries lock while calling `make_query`"

This reverts commit fd3d2d4.

This has the side effect, that when Clippy should ICE (during an EarlyPass?) it will fill up the RAM with 2 GB/s and then freezes the PC. I don't know the correct solution, but this is blocking the Clippy sync and might give some people really bad experiences, so this should be reverted ASAP.

Reverts rust-lang#112333

r? `@cjgillot`
cc `@Zoxc`

I only commented this on [Zulip](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/.60try_print_query_stack.60.20has.20.60ImplicitCtx.60.20during.20.60EarlyPass.60/near/363926180). I should've left a comment on the PR as well. My bad.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jun 23, 2023
…ce, r=cjgillot

Revert "Don't hold the active queries lock while calling `make_query`"

This reverts commit fd3d2d4.

This has the side effect, that when Clippy should ICE (during an EarlyPass?) it will fill up the RAM with 2 GB/s and then freezes the PC. I don't know the correct solution, but this is blocking the Clippy sync and might give some people really bad experiences, so this should be reverted ASAP.

Reverts rust-lang#112333

r? `@cjgillot`
cc `@Zoxc`

I only commented this on [Zulip](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/.60try_print_query_stack.60.20has.20.60ImplicitCtx.60.20during.20.60EarlyPass.60/near/363926180). I should've left a comment on the PR as well. My bad.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 23, 2023
…ce, r=cjgillot

Revert "Don't hold the active queries lock while calling `make_query`"

This reverts commit fd3d2d4.

This has the side effect, that when Clippy should ICE (during an EarlyPass?) it will fill up the RAM with 2 GB/s and then freezes the PC. I don't know the correct solution, but this is blocking the Clippy sync and might give some people really bad experiences, so this should be reverted ASAP.

Reverts rust-lang#112333

r? `@cjgillot`
cc `@Zoxc`

I only commented this on [Zulip](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/.60try_print_query_stack.60.20has.20.60ImplicitCtx.60.20during.20.60EarlyPass.60/near/363926180). I should've left a comment on the PR as well. My bad.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

5 participants