offers the synchronous part of start and stop in IHostedService to run concurrently #85191
Conversation
ghost
added
the
community-contribution
|
Tagging subscribers to this area: @dotnet/area-extensions-hosting Issue DetailsContributes to #68036
|
| _ = TryExecuteBackgroundServiceAsync(backgroundService); | ||
| } | ||
| })); | ||
| Task tasks = Task.WhenAll(_hostedServices.Select(service => Task.Run(() => StartAndTryToExecuteAsync(service, combinedCancellationToken)))); |
There was a problem hiding this comment.
Should we be passing token into Task.Run as well?
| Task tasks = Task.WhenAll(_hostedServices.Select(service => Task.Run(() => StartAndTryToExecuteAsync(service, combinedCancellationToken)))); | |
| Task tasks = Task.WhenAll(_hostedServices.Select(service => Task.Run(() => StartAndTryToExecuteAsync(service, combinedCancellationToken), combinedCancellationToken))); |
| _ = TryExecuteBackgroundServiceAsync(backgroundService); | ||
| } | ||
| })); | ||
| Task tasks = Task.WhenAll(_hostedServices.Select(service => Task.Run(() => StartAndTryToExecuteAsync(service, combinedCancellationToken), cancellationToken))); |
There was a problem hiding this comment.
Why does this use cancellationToken rather than combinedCancellationToken as the Task.Run argument?
Also, since this isn't a hot code path I assume we're ok with the extra allocation that's incurred here for the closures and whatnot?
There was a problem hiding this comment.
Why does this use cancellationToken rather than combinedCancellationToken as the Task.Run argument?
I fixed the combined cancellation tokens, that was an error from me.
Also, since this isn't a hot code path I assume we're ok with the extra allocation that's incurred here for the closures and whatnot?
However I don't know if eliminating the closure in Task.Run is good. I know that Task has constructor that accepts Action<object?> action, object? state, CancellationToken cancellationToken. and that Task.Factory.StartNew has the same call. However I am unable to weight the gains of reducing a closure vs the loss from a (back and forth)cast from object to CancellationToken.
| @@ -185,7 +182,7 @@ public async Task StopAsync(CancellationToken cancellationToken = default) | |||
|
|
|||
| if (_options.ServicesStopConcurrently) | |||
| { | |||
| Task tasks = Task.WhenAll(hostedServices.Select(async service => await service.StopAsync(token).ConfigureAwait(false))); | |||
| Task tasks = Task.WhenAll(hostedServices.Select(service => Task.Run(() => service.StopAsync(token), cancellationToken))); | |||
There was a problem hiding this comment.
Same question here about why two different tokens are being used?
There was a problem hiding this comment.
Thanks @pedrobsaila LGTM.
@eerhardt @stephentoub do you have any other feedback?
|
Failures unrelated and looks all known issues |
|
All failures are known issues, merging |
Contributes to #68036