Retry running generators if the project config looks good, but the source generator doesn't produce host outputs

[davidwengier]

Fixes the startup issue we have if Razor loses the race condition, and Roslyn caches the generator run result while it's in "cohosting off" mode.

Essentially our snapshot manager, which is our first port of call for "I would like to run this OOP service for this solution snapshot" will not sometimes redirect calls to a forked snapshot, that ensures the generator will have had a chance to re-run after it has seen the cohosting flag turn on. Some minor changes to solution and project snapshots because this could happen while code is in-flight, but after the first requests are done, subsequent requests flow through the system as normal (albeit being silently redirected to the fork) and once almost any change to the project or additional files has happened, the whole "retry project" notion becomes unnecessary.

Val build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2793362&view=results
Test insertion: https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/670224

[DustinCampbell]

This change makes me a bit uncomfortable. It breaks the entire notion of a ProjectSnapshot being immutable, and that doesn't feel right. I worry that there are subtle bugs hiding in this change.

[chsienki]

This is definitely funky :) but we only need this while cohosting on/off is an option right? If we are always in cohosting mode then suppression is no longer a thing, and we don't need to do any of this dance.

We should still think if there is a better way to fix the generator, so this isn't necessary. But I think for expediency we should take this now, and we can back it out if/when we find a better solution.

[davidwengier]

I'll be generous and say this is "pragmatic", but I don't disagree with either of your takes either :)

On being mutable, yes it's annoying but I think its a necessary step. I see the snapshot manager "mutation" as akin to workspace.TryApplyChanges in the devenv world. The mutation in RemoteProjectSnapshot is obviously more annoying, but in my mind (and with my thinking through the scenarios, I definitely could have easily missed something) its okay because a) We only go from normal to retry project, never the other way, and that is protected by the lock in the snapshot manager, b) we're making a semantically meaningless mutation so the failure mode is an extra generator run, not a change of output, and c) to do it properly means bubbling the retry all the way back out to whoever first got the snapshot, having them get a new snapshot and trying again. Doing that without exceptions-as-control-flow seemed like a nightmare. Happy to add a lock though, of course :)

And the final reason is yes, as far as I'm aware this is temporary until cohosting can just be the way things work. This doesn't even actually fix the initialization issue that we can see (where Find All Refs doesn't consider Razor docs until one is opened), it is just to get us to be working so we can ship in preview 2.

[chsienki]

I really want there to be a better way to do this, but I've spent all morning trying to come up with something, and everything I came up with is more hacky than this anyway :/

[davidwengier]

I have restored the immutability of the project snapshot, by extracting the run result out to its own class, that wraps the actual generator run result host output, and the project it comes from. This means the snapshot doesn't need to change any more, it's just that if you need to access generator results from it, it will always pull from the correct place, and the snapshot managers lock protects that.

[davidwengier]

Given the preview 2 deadline, I'm going to go ahead and merge this, so we can be in a position to make a decision on preview 2 before snap.