"Edit and continue" errors are not cleared from the document after i stop debugging

[DoctorKrolic]

Version used: VS 2022 v17.3 Preview 2

To reproduce:

1. Start debugging blazor app
2. Make change, that cannot be taken by hot reload
3. Observe "Edit and continue" error
4. Stop debugging

Expected behavior: The error is gone after I stop debugging

Actual behavior: The error is gone only if I reopen the document

Demo:

[davidwengier]

This should have been fixed with https://github.com/dotnet/roslyn/pull/60081, will have to investigate

[davidwengier]

@dibarbet are there any known issues with pull diagnostics in 17.3 P2? I can repro this, but the EnC version stuff I added seems to still be working okay, from a quick look.

[dibarbet]

@dibarbet are there any known issues with pull diagnostics in 17.3 P2? I can repro this, but the EnC version stuff I added seems to still be working okay, from a quick look.

There have been some platform issues, but I don't remember one like this. Might need to try on p3

[davidwengier]

Nope, found the issue I think. When ending the debug session, documentsToReanalyze is coming back as empty. Any ideas or recent changes you can think of @tmat, while I keep trying to debug?

[davidwengier]

Okay, we're tracking the document properly in the edit session, but when ending the session, its not the same session, as there is a BreakStateChanged call before that, so we actually restart the edit session just before we end it. Don't know if thats new?

When restarting though, we do seem to the do the right thing and bump version numbers etc. but the diagnostics don't get cleared. Not sure what is going on

[davidwengier]

Okay so when we get the break state changed, we know there is a document to reanalyze, but the reanalysis does nothing, because the document ID is not in the project that the work coordinator is running on. When we get the end session, there aren't any documents to analyze, because we've cleared them after the break state change.

I'm guessing this is a design time document issue of some kind, because we're asking the solution crawler to analyze Pages_Counter_razor.g.cs but the project has Counter.razor.g.cs in it

[tmat]

@davidwengier Thanks for the investigation. Does this repro when using pull diagnostics?

[davidwengier]

Yes, if I turn on the pull diagnostics flag in Tools, Options, it repros in a normal C# file too

[davidwengier]

I think there might be two issues here.. I can fix the issue in normal C# files with pull diagnostics turned on (fix in https://github.com/dotnet/roslyn/pull/62270) but the issue remains in Razor if pull diagnostics is turned off. I thought pull diagnostics was always on for Razor?

[davidwengier]

Fixed in #62270

[DoctorKrolic]

@davidwengier I recently faced this issue again in 17.3. I then checked it in 17.4 Preview 1 and it is still there! I don't see any reason to publish another gif, because I did the same steps as in issue description and got absolutelly the same results. Can you please reopen this issue (it was closed by you, so I cannot reopen it myself) and reinvestigate it. I am open to provide any additional data if required.

[davidwengier]

Yep, I can repro this too. Thanks

[davidwengier]

Looks like my code is still working fine, and the Roslyn LSP handlers seem to be doing the right thing too. Will have to keep digging.

[davidwengier]

Current working theory is that with the solution crawler disabled for pull diagnostics, EnCs call to Reanalyze in order to clear rude edits is essentially no-oping, so whilst the version incrementing code I wrote is working, and we're sending back a new diagnostics response, that response still contains the old diagnostic because the document hasn't changed.

[davidwengier]

err.. I think it gets worse.. I'm not getting any rude edits in 32904.27.main which makes this very hard to debug. For a C# console app I get the errors, but they're all on line 1. For Razor I get nothing, but that probably makes sense if they're all on line 1, since we can't map that location.

[davidwengier]

...and now I can't repro this issue, or the rude edit issue above, under the debugger.. so I'm guessing there is a race somewhere.

@mavasani @tmat any pro tips for what to look for here? or even how to debug it?

The story, as far as I can tell, goes like this:

1. Start debugging
2. Introduce rude edit
3. Rude edit is reported by EditAndContinueDiagnosticAnalyzer, which doesn't report to EditAndContinueDiagnosticUpdateSource (potential fix 1)
4. Stop debugging
5. We get a BreakStateOrCapabilitiesChangedAsync, and know which documents to reanalyze, and ask for them to be reanalyzed
   1. These are compile-time documents, not design time documents, which might be part of the problem for Razor, but the problem repros for a console app too (potential fix 2)
   2. We don't increment the version in EditAndContinueDiagnosticUpdateSource because we don't know any were reported (potential fix 3)
6. We get a EndDebuggingSessionAsync, with no documents to analyze (because its a different session from above)
   1. We do increment the version in EditAndContinueDiagnosticUpdateSource because we're ending the session

Seemingly at any point in the above we might get a pull diagnostics request, and it seems that when that happens, even if we've bumped the version in EditAndContinueDiagnosticUpdateSource, sometimes the rude edit still comes back from the diagnosticSource.GetDiagnosticsAsync call.

It seems to me like the Reanalyze call is not blocking, and since the document content didn't change, perhaps that call is just returning cached data. I haven't been able to debug through to verify this, because debugging changes the timing of everything so much that either the problem doesn't repro, or diagnostics is so delayed I can't trust anything anyway. Unfortunately the timing also means its hard for me to confirm whether any of the fixes actually work. I suspect they don't, and what we need is a way to invalidate the diagnostics cache, rather than (or as well as) calling Reanalyze, but I have no idea where that would go.

Thoughts?

[tmat]

+ @CyrusNajmabadi

I feel like we should declare fixing this lost cause and wait until we can fully switch to pull diagnostics.

[davidwengier]

Is there some plan for how pull diagnostics will save us all? Because this is all reproing now with pull diagnostics on...

[tmat]

The current implementation is not using pull diagnostics directly. It's going thru the legacy diagnostic analyzer code paths.