-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BackgroundService blocked the execution of whole host #36063
Comments
Fixed by adding |
Yeah the method shouldn't be blocking. Wrapping in a |
Reopening this so we can look at it. I'm glad that you solved your problem but we should try to solve this so others don't hit it. |
It looks like if your run a bunch of CPU-bound code in In health checks I ended up inserting a |
Async method executes synchronously till first |
But it would be great if you can solve this case |
It's a trade off. There may be reasonable initialization code the service needs to run before letting the app continue. It's within the service's control how long it blocks. |
I have also encountered this issue and solved it in the same way as @vova-lantsov-dev did. However, my main concern is that this behavior is not straightforward, would be nice if this was more predictable or documented somewhere |
I think it's reasonable to dispatch public class MyService: BackgroundService
{
public override async Task StartAsync()
{
await InitializeAsync();
await base.StartAsync();
}
public override async Task ExecuteAsync()
{
// Do background stuff...
}
} It's called BackgroundService. It seems odd that you can block the foreground in the default case. If we're not happy with overriding This would be a breaking change however, since existing services may depend on the initial synchronous work blocking the startup process. |
Sorry I never did this change but enough people have hit it now I think we should just do it. https://github.com/aspnet/Extensions/tree/davidfowl/background-service |
I just ran into this and was about to leave feedback on the docs. Glad I checked here first. The Worker template works because it awaits a Task.Delay. Change that to a |
@davidfowl I just looked at the BackgroundService changes in your branch - would it make more sense for the Host to Task.Run each hosted service instead of depending on the hosted service to do it? |
I'm not a fan as it's wasteful, but if that many people run into it it might be worth doing. I'd honestly rather teach people that these things aren't running on dedicated threads so blocking them isn't the way to go. |
Triage summary: The work here is to consider dispatching |
It was very helpful to learn from the updated docs that "No further services are started until ExecuteAsync becomes asynchronous, such as by calling await." docs Is there a possibility of this being addressed in .NET Core 3.1? |
This is how I resolved it in my case [as detailed in 17674 referenced above]: I started from scratch, choosing to implement my own version of it using This works quite well in my setup and actually sped up the app start time [compared to even using the Task.Yield() workaround as mentioned above]. |
For what it's worth: The argument that you can just override This: class MyService : BackgroundService
{
protected override async Task ExecuteAsync(CancellationToken ct)
{
var state = ...;
await Task.Yield();
while (true)
{
Work(state);
await Task.Delay(..., ct);
}
}
} Turns into this: class MyService : BackgroundService
{
T? _state;
public override Task StartAsync(CancellationToken ct)
{
_state = ...;
return base.StartAsync(ct);
}
protected override async Task ExecuteAsync(CancellationToken ct)
{
while (true)
{
Work(_state!);
await Task.Delay(..., ct);
}
}
} This seems like a significant regression in code clarity to me. There's also an argument to be made that, if the current behavior of The name |
@davidfowl is your commit here still a true WIP or are you redacting your idea to go this route? I found the documentation here that references this issue a little confusing as it initially made me think that even using I believe any confusion would be mitigated by doing what you initially proposed in that commit and having the abstraction dispatch our executing |
I couldn't figure out the best area label to add to this issue. Please help me learn by adding exactly one area label. |
I noticed that hangs can now occur in later versions of ASP Core if you do a Task.Delay. Previous versions of ASP Core were not causing this issue but I don't have exact version information. Calling |
@HelloKitty |
@vova-lantsov-dev It appears so, but I do not think this was the case in a previous version of ASP Core. Maybe 2.1. This ran fine without hanging a year ago, anyway I have adopted |
This causes an issue when unit testing. If I mock a service to return a [TestClass]
public class TestClass
{
[TestMethod, TestCategory("UnitTest")]
public async Task Background_Service_Should_Return_FromStartAsync()
{
var sut = new FakeService();
await sut.StartAsync(Token);
//code never reaches here because ExecuteAsync never awaits a task
Assert.IsTrue(true);
}
}
public class FakeService : BackgroundService
{
protected override async Task ExecuteAsync(CancellationToken stoppingToken)
{
while (!stoppingToken.IsCancellationRequested)
{
await Task.CompletedTask;
}
}
} |
Look at the implementation of
Because |
Another problem with this behaviour is with exceptions. When the service throws a exception in the first line the process is stopped even if public class ServiceWithError : BackgroundService
{
//protected override Task ExecuteAsync(CancellationToken stoppingToken) => Task.Run(() => throw new System.NotImplementedException());
protected override Task ExecuteAsync(CancellationToken stoppingToken) => throw new System.NotImplementedException();
} |
This issue hasn't been planned for 7.0 (see #64015). Moving to Future. |
I would like to point out that the "executes synchronously till first protected override async Task ExecuteAsync(CancellationToken stoppingToken)
{
using var timer = new PeriodicTimer(TimeSpan.FromHours(24));
// If I do not add following line, the code stops for very long time. More than 1 hour.
await Task.Yield();
do
{
logger.Debug($"Timer triggered.");
var count = Interlocked.Increment(ref executionCount);
logger.Debug($"Trigger counter:{count}.");
// this await wont return control back to main program
// it is still blocking
await RunLoop(stoppingToken);
} while (await timer.WaitForNextTickAsync(stoppingToken));
}
private async Task RunLoop(CancellationToken stoppingToken)
{
foreach (var indexType in Enum.GetValues<AutocompleteIndexType>())
{
logger.Debug($"Creating {indexType:G} index.");
try
{
// this await wont return control back to main program
// it is still blocking - probably because 90 % of following calls are "synchronous" inside
await IndexFactory.CreateCacheIndex(indexType);
}
catch (Exception e)
{
logger.Error($"Error occured during {indexType:G} index creation.", e);
}
}
logger.Debug("Running delete of old indexes.");
try
{
IndexFactory.DeleteOld();
}
catch (Exception e)
{
logger.Error($"Problem occured when deleting old files.", e);
}
} But |
I just ran into this issue, and in my case it seems that
|
So, I tried the above code, and apparently the issue only happens on Mac, on both vscode and vs for Mac. |
Yes, can confirm this message. Debug build in Rider on Mac blocks, Run in Rider on Mac works. |
We have simple Backround service like this: `
}` And it B L O C K S the startup of the server for 1 minute.... This is not how should have the BackgroundService and ExecuteAsync behave... I have found article about this strange behavior here: but.. somehow this important issue died? |
Maybe we should have 2 class of background service: |
I have the same issue, but I noticed that this issue happens only when I run the project using the command line using "dotnet run", but when using the visual studio code or visual studio, it runs successfully, and the background service is working alongside the API, I test the API using postman and it works. |
I've run into this too. |
I think this should make it in NET 8.0 as indicated here |
The |
Sad that even in v8 this won't be fixed. For anyone who lands here, see Stephen Cleary's blog series which provides workarounds for these issues. |
.NET 8 has been released, and unfortunately, Microsoft has turned a blind eye to this issue. The situation is amusing - to start a synchronous service, you need to await something, for example, Task.Yield, resulting in the generation of a state machine and a loss of at least 0.05% performance at this stage. |
Yeah.. MS is way more focusing on implementing YET next type of UI library, just to deprecate and kill it 1 year later, rather then solve these issues.. |
I’m very confused why Task.Run isn’t a viable workaround? |
You can do that until the presence of a state machine becomes a hindrance to you, and the TaskScheduler works perfectly. |
When does that happen? |
Most developers don't think about |
This sounds like an unnecessary "rite of passage" pitfall for anyone deciding to stroll into BackgroundService country. Myself twice now (2020 + 2024) 😆 |
Nothing here was working for me...🔴 As it turns out you cannot test this with breakpoints! Maybe that seems logical to some people, but it did not dawn on me until I spent an hour, tried every single solution and workaround in this issue, and finally said, to myself, "Hmmm, maybe I'll try logging instead of using a breakpoint". Give me a thumbs up if this was helpful. And give me a pie in the face otherwise! 🤣 |
this sounds more like a edjucation/knownledge or documentation issue then. Just slapping async/await onto everything is not "working with tasks". |
Everything should've been built in a foolproof way. |
Hit this issue again. It takes a long time to diagnose the issue and arrive here for various workarounds. If one doesn't have guru-level async knowledge, it's an insidious time waster. Please please please consider this for v9. 🙏 |
Describe the bug
When I run the specific
foreach
cycle inBackgroundService
-derived class, it blocks the whole host from starting. Even second hosted service doesn't start. When I commentforeach
cycle, everything works as expected.To Reproduce
TargetFramework: netcoreapp2.1
Version: 2.1.12
Use following hosted service: NotificationRunner.cs
And singleton: NotificationService.cs
Expected behavior
BackgroundService.ExecuteAsync should work in background without blocking even if it has blocking code. As you can see in NotificationService class, cancellation of enumerator is based on IApplicationLifetime.ApplicationStopping, but anyway it shouldn't affect the host startup because BackgroundService is expected to run in background :)
Screenshots
When
foreach
cycle existsThen execution is blocked on this cycle
But when
foreach
cycle is commentedThen execution continues as expected
(But why twice?)
Additional context
The text was updated successfully, but these errors were encountered: