Uh oh!
There was an error while loading. Please reload this page.
Ensure BackgroundService invokes ExecuteAsync after start - #132241
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3dfd3a15-eb10-455b-8c1c-16ecd87fd841
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
steveisok
commented
Aug 12, 2026
@jeffhandley deferring to you on who best to review. |
There was a problem hiding this comment.
Pull request overview
This PR adjusts BackgroundService.StartAsync scheduling so that ExecuteAsync is still invoked even if the service is stopped/disposed immediately after start, and adds regression coverage to validate the behavior under deterministic thread-pool starvation.
Changes:
- Update
BackgroundService.StartAsyncto avoid using the stopping token as theTask.Runscheduling token. - Add a regression test that blocks the sole thread-pool worker and verifies
ExecuteAsyncstill runs exactly once when immediately stopped/disposed. - Update the pre-canceled
StartAsynctest to assertExecuteTaskis canceled andExecuteAsyncis not invoked.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/Microsoft.Extensions.Hosting.Abstractions/src/BackgroundService.cs | Changes how ExecuteAsync is scheduled/canceled to prevent cancellation from suppressing delegate invocation. |
| src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/BackgroundServiceTests.cs | Adds deterministic regression coverage for immediate stop/dispose and refines pre-canceled start assertions. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3dfd3a15-eb10-455b-8c1c-16ecd87fd841
There was a problem hiding this comment.
Review details
Suppressed comments (2)
src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/BackgroundServiceTests.cs:203
ExecuteInvocation.GetResult()has no timeout, so ifExecuteAsyncis never invoked this test will hang the RemoteExecutor process rather than failing with an assertion. Add a bounded wait (reusingRemoteExecutor.FailWaitTimeoutMilliseconds) before reading the result.
(int invocationCount, int threadId, bool isThreadPoolThread, bool isCancellationRequested) =
service.ExecuteInvocation.GetAwaiter().GetResult();
Assert.Equal(1, invocationCount);
src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/BackgroundServiceTests.cs:163
- The RemoteExecutor child process can hang indefinitely if the regression is reintroduced (or if the child process stalls). Similar tests in this repo set
RemoteInvokeOptions.TimeOutso a hang fails fast and doesn't consume the full harness timeout.
This issue also appears on line 201 of the same file.
var options = new RemoteInvokeOptions();
options.StartInfo.EnvironmentVariables["DOTNET_ThreadPool_UseWindowsThreadPool"] = "0";
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
rosebyte
left a comment
There was a problem hiding this comment.
I'm not sure I follow the wording about preserving .NET 10 behaviour. Do you mean that this preserves only the .NET 10 change whereby all ExecuteAsync work runs asynchronously on a thread-pool thread? As I understand the file history, invocation itself has had three different behaviours:
Before .NET 10,
ExecuteAsyncwas called regardless of whether the cancellation token was already cancelled.In .NET 10, whether
ExecuteAsyncis called effectively depends on whether the scheduler wins the race with cancellation.With this PR,
ExecuteAsyncis called if and only if the cancellation token has not already been cancelled whenStartAsyncchecks it.
The PR makes sense to me, and I consider the resulting behaviour clearly superior, so I'm going to approve it. I only wonder if I miss anything in the PR descriptiion.
steveisok
commented
Aug 20, 2026
@rosebyte before moving this forward I want to bring #131249 (comment) here. He's got a fair concern and there is a trade-off we are making as a result of the PR. We are trading “sometimes silently never invoke the service” for "always invoke it, requiring the service to honor an already-canceled token.”. Thoughts on what you think is best? |
rosebyte
commented
Aug 20, 2026
Passing |
steveisok
commented
Aug 21, 2026
/ba-g Known issue #132336 |
Uh oh!
There was an error while loading. Please reload this page.
steveisok
commented
Aug 21, 2026
/backport to release/11.0-rc1 |
Started backporting to |
…r start (#132598) Backport of #132241 to release/11.0-rc1 /cc @steveisok ## Customer Impact - [ ] Customer reported - [ ] Found internally [Select one or both of the boxes. Describe how this issue impacts customers, citing the expected and actual behaviors and scope of the issue. If customer-reported, provide the issue number.] ## Regression - [ ] Yes - [ ] No [If yes, specify when the regression was introduced. Provide the PR or commit if known.] ## Testing [How was the fix verified? How was the issue missed previously? What tests were added?] ## Risk [High/Medium/Low. Justify the indication by mentioning how risks were measured and addressed.] **IMPORTANT**: If this backport is for a servicing release, please verify that: - For .NET 8 and .NET 9: The PR target branch is `release/X.0-staging`, not `release/X.0`. - For .NET 10+: The PR target branch is `release/X.0` (no `-staging` suffix). ## Package authoring no longer needed in .NET 9 **IMPORTANT**: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version. Keep in mind that we still need package authoring in .NET 8 and older versions. Co-authored-by: Steve Pfister <steveisok@users.noreply.github.com> Copilot-Session: 3dfd3a15-eb10-455b-8c1c-16ecd87fd841
Fixes#131249.
BackgroundService.StartAsynccurrently passes its stopping token toTask.Runas the scheduling token. IfStopAsyncorDisposecancels that token before the queued delegate begins, the task transitions toCanceledwithout invokingExecuteAsync.This makes invocation timing-dependent: after a non-canceled start is accepted,
ExecuteAsyncmay or may not run depending on whether the thread pool dequeues it before cancellation.This change:
ExecuteAsyncwork runs asynchronously on a thread-pool thread.CancellationToken.Noneas theTask.Runscheduling token.ExecuteAsync, so an immediate stop or dispose invokes it with cancellation already requested.StartAsyncbehavior by explicitly assigningTask.FromCanceled(cancellationToken)toExecuteTaskwithout invokingExecuteAsync.Regression tests deterministically occupy the sole thread-pool worker and verify that immediate stop and dispose still invoke
ExecuteAsyncexactly once on a thread-pool thread with an already-canceled stopping token. Pre-canceled startup coverage verifies thatExecuteTaskis canceled andExecuteAsyncis not invoked.Note
This pull request description was generated with GitHub Copilot.