Fix race in GetOpenDocumentInCurrentContextWithChanges

[jasonmalinowski]

This was reading Workspace.CurrentSolution twice, so if the document was removed between two reads it'd end up throwing exceptions or triggering asserts.

Fixes #33652

Ask Mode template

Customer scenario

Sometimes when you're using C# or VB code in Visual Studio and you have code lens on, it'll crash.

Bugs this fixes

#33652

Workarounds, if any

Turning off CodeLens will reduce the risk.

Risk

Zero. Trivial bug fix that closes a race.

Performance impact

Zero.

Is this a regression from a previous update?

Yes, ish. The bug has existed for ages, but some changes in 16.0 has probably made it easier to hit.

Root cause analysis

There's a longstanding race in a common helper function that's used in almost every Roslyn feature. The race can happen if a thread is updating the state of our projects while another thread is calling this function. I suspect we've never really noticed this because usually those were always both the UI thread. But in 16.0, we:

1. Refactored some Code Lens stuff; I haven't checked but this means this function is currently being called on a background thread, and it's possible that moved and is more likely to be hit.
2. Refactored our project system handling, which means our project state can also be updated on a background thread.

How was the bug found?

Internal automated test runs.

[jasonmalinowski]

Turns out this fixes #33652.

[sharwell]

@jasonmalinowski I believe this occurred while testing in a 16.0 branch. It might be worth taking there since it's also one of our top hitters in integration tests.

[sharwell]

Integration test failure was caused by #34197. Re-running...

[jasonmalinowski]

@sharwell, taking this in 16.0 would mean seeking QB approval and I'm not sure it meets the bar there since it's product code changing. It is undoubtedly a bug, but and the risk of the fix is zero though.

@jinujoseph let's discuss this on Monday. I don't have a strong feeling of where we take this, but the bar application is pretty weird in this case.

[jinujoseph]

@jasonmalinowski can you fill out the ask mode template and let's do a barcheck for 16.0

[jasonmalinowski]

@jinujoseph: moved this back to dev16.0 and filled out the ask mode template. I'm for taking it, if only because the risk of not fixing it is greater than the risk of fixing it. (i.e. this is negative risk!)