Progress messages logic might have race conditions

[ndmitchell]

I'm not familiar with the current progress code, so the below is somewhat guesswork and assumption - happy to understand why I'm wrong. There seem to be two invariants around progress messages:

1. During testing we expect progress messages to be sent. If we don't, the benchmarking fails.
2. The LSP spec requires that no progress messages be sent between initialise request/response, as per https://microsoft.github.io/language-server-protocol/specifications/specification-3-15/#initialize

As far as I can tell, the way we ensure invariant 1 is by forking a progress thread and not doing a sleep at the beginning. Unfortunately, if you make the operations fast enough or lose a bunch of races, it doesn't get a progress notification, and we get failures. I'm seeing a failure with the ghcide benchmarking stuff I think comes from this.

As far as I can tell, the way we ensure invariant 2 is by hoping the initialise gets handled super quickly, and that the timer gets killed. Again, that's using sleeps and races to ensure the property.

I've changed a lot of timing properties around the Shake graph, and am suddenly getting a lot of errors from progress stuff. While these might be due to my changes, my debugging suggests progress itself might be at fault.

[pepeiborra]

Invariant 1 is indeed an assumption for many tests. It would make sense to have an entirely separate codepath for testing that doesn't spawn a separate thread and instead sends progress notifications directly. Let me look into that.

The proper way to enforce invariant 2 would be to make the request handler synchronous. We already handle notifications synchronously, for instance. I'll look into that too.