-
Notifications
You must be signed in to change notification settings - Fork 786
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
Fix potential OCE in graph processing #17921
Conversation
|
Oh I see you're addressing this in a sibling PR, sorry, reviewing this in order :D |
Yes I should mention it here, my guess is that failing test is unrelated, I just introduced flakiness recently and it's coincidental that it's about cancellation. |
I've seen Rider hanging in a situation that looks like could be caused by this issue - indefinitely waiting for some FCS work that never happens, according to the debugger. It's a ~400 projects mixed C# & F# solution with parallel projects analysis enabled, so I can imagine race conditions are more likely than in smaller codebases. So I'm definitely cheering for this to get merged 👍 |
Is Rider using Graph checking for analysis somehow? |
Is it plumbed in checker at all? |
Only through Transparent Compiler. |
Yeah. I don't know if Rider exposes it. Probably @auduchinok should know. |
Now I think my analysis here is rubbish, because |
Surveying the code base for unobserved background jobs that may throw I noticed this
Async.Start
that takes a cancellation token but is not effectively catching cancellations. Theoretically it is possible that the cancellation occurs beforeprocessNode
starts, resulting in an unhandled OperationCancelledException.This could potentially impact Transparent Compiler (?), I might have also observed it when running tests concurrently in #17872.