dotnet / project-system

The .NET Project System for Visual Studio
MIT License
972 stars 389 forks source link

Improve cancellation support in LaunchSettingsProvider #7056

Open tmeschter opened 3 years ago

tmeschter commented 3 years ago

The LaunchSettingsProvider handles concurrent modifications by queuing them up in a SequentialTaskExecutor where they are processed one at a time. Once a Task is queued up there is no way of cancelling it, with the exception that disposing the LaunchSettingsProvider will cancel all pending Tasks.

We should looking into updating LaunchSettingsProvider (and ILaunchSettingsProvider) to accept and respect CancellationTokens.

drewnoakes commented 3 years ago

It should probably accept CancellationToken instead of the current timeout values. It's possible to model the latter with the former.

drewnoakes commented 3 years ago

It should probably accept CancellationToken instead of the current timeout values. It's possible to model the latter with the former.

7581 addresses this for internal code via an extension method. Unfortunately the interface is public so cannot be changed. We may like to consider making the extension method public. We may also need to version the API so that the cancellation token can be passed to internals (which is not currently done in the extension method).

drewnoakes commented 2 years ago

We should consider replacing the use of SequentialTaskExecutor with a dataflow block. We are pulling items off a datasource and queuing them in the executor. It'd likely be simpler to just use dataflow throughout.

That said, it's unclear to me what would trigger a cancellation here. ILaunchSettingsProvider seems like a place we could have accepted cancellation tokens, but it's a public API and hard to change now.