Hey, we've seen this assertion trigger in rust-analyzer a number of times: https://github.com/rust-lang/chalk/blob/df09cc603c0cff9ed95e5554055d483fdd756fbc/chalk-engine/src/logic.rs#L106, the most recent issue is https://github.com/rust-analyzer/rust-analyzer/issues/1927.
The failure seems somewhat non-deterministic (it only triggers if invoke compiletion in a particular way). Does this assert means some internal problem in chalk, or are we holding it wrong?
mmmmmmmmmm I wonder
it could be perhaps that salsa is panicking
and that chalk is not panic-safe
(I bet that's correct)
e.g., if there were a panic in one of the callbacks that occurred when solving someting
we'd probably leave things in
and then if you call back in, you'll see that error @matklad
I can open an issue with some notes on what it would take to fix, probably do-able
sounds plausible. When we panic due to cancellation, we advance the revision, which will cause chalk's engine to be re-created (b/c its a volatile query), however there's still a window of opportunity to observe chalk in this revision
I've just tried to reproduce this without cancellation, and I see 30 seconds requestsion, but now panics. So, the plausible narrative is that, in this case, chalk takes too long to infer something (which is an unrelated problem). However, because it's slow, it's likely to observe cancellation, which additionally triggers a panic. I think we can easily do a work-around on our side, by using a mutex with poisoning around the
chalk_solve::Solver. We currently use
Heh, I am looking into why
UnwindSafe didn't caught this issue, and I think that we didn't fix that cyclic bound problem in salsa, and instread I've just put AssertUnwindSafe around db
overflow evaluating the requirement db::RootDatabase: ra_hir::db::HirDatabase, my old friend...