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

use mailbox processors per-file to serialize diagnostics pushes #803

Merged
merged 3 commits into from
Jun 20, 2021

Conversation

baronfel
Copy link
Contributor

Right now each publish of diagnostics for a file is fired off via Async.Start into the void. This can result in ordering issues with the state of each files' diagnostics collection.

This PR serializes publishing of diagnostics per-file by using a mailboxprocessor as a synchronization agent per-file, with error logging and restarts. The state of the file is maintained in a map that's returned recursively, and this way each individual publish action for a file is well-ordered with respect to that file.

@baronfel
Copy link
Contributor Author

One thing I have noticed here is that if the MBP errors, we'll start a new agent for that file, but we don't currently have a way of extracting the state from the already-running agent. We can't simply use a new message with a reply for that case, though, because the agent will be in an error state and (I believe) not responding to new messages.

@baronfel
Copy link
Contributor Author

I'd like @Krzysztof-Cieslak's thought on this approach overall, but I'm pretty happy with it!

@baronfel baronfel force-pushed the diagnostics-clobbering branch from 6d29679 to a51104e Compare June 20, 2021 18:31
Copy link
Member

@Krzysztof-Cieslak Krzysztof-Cieslak left a comment

Choose a reason for hiding this comment

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

LGTM

@baronfel baronfel merged commit 492c4de into ionide:master Jun 20, 2021
@baronfel baronfel deleted the diagnostics-clobbering branch June 20, 2021 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants