Skip to content
This repository has been archived by the owner on Dec 19, 2018. It is now read-only.

WebHost.Run() completes before ApplicationStopping #812

Closed
ZoolWay opened this issue Jul 12, 2016 · 10 comments
Closed

WebHost.Run() completes before ApplicationStopping #812

ZoolWay opened this issue Jul 12, 2016 · 10 comments
Assignees

Comments

@ZoolWay
Copy link

ZoolWay commented Jul 12, 2016

When using applicationLifetime.ApplicationStopping.Register(HandleStopping); the program exits host.Run(); before that handler has completed also the documentation (IntelliSense) says Shutdown will block until this event completes.
This also means that ApplicationStopped will be executed before ApplicationStopping has completed.

Also it should be noted that ApplicationStopping is not executed on the main thread (while ApplicationStopped is) and that you should not use async for those handler as that will cause the methods to be completed.

This is ASP.NET 1.0.0 on Windows 10 using Kestrel (1.0.0).

Startup.cs

public void Configure(IApplicationLifetime applicationLifetime)
{
    applicationLifetime.ApplicationStopping.Register(HandleStopping);
    applicationLifetime.ApplicationStopped.Register(HandleStopped);
}

private void HandleStopped()
{
    System.Threading.Thread.Sleep(30000);
    // WebHost.Run() will not end until this method completes, running it in the main thread
}

private void HandleStopping()
{
    System.Threading.Thread.Sleep(30000);
    // WebHost.Run() will end even while this method is still working, it will even run HandleStopped before this completed. It run on a worker thread.
}
@davidfowl
Copy link
Member

Yep I can reproduce the problem.

@guardrex
Copy link
Contributor

Not directly related, but speaking of doing things on "ProcessExit" in case someone is looking around at various options, see: https://github.com/dotnet/corefx/issues/10012#issuecomment-232436326

@mrahhal
Copy link
Contributor

mrahhal commented Oct 28, 2016

Is there anything we can do about this for now? Can't depend on 1.1.0-preview1 for my app that is running in production. Had to work hard to track down this issue as I've been depending on IApplicationLifetime and started noticing background tasks abruptly exiting without notice.

Thought about doing some manual sync that makes Stopped wait for all Stopping, but since services are being disposed directly, all tasks that depend on other services will probably face a lot of "object disposed exceptions" in all cases. Hmm this really looks hopeless without including a custom build of this package.

@davidfowl
Copy link
Member

It might be worth putting this fix in a patch release.

/cc @Eilon @muratg @DamianEdwards

@davidfowl
Copy link
Member

@mrahhal what problem is it causing you right now? What does your repro look like?

@mrahhal
Copy link
Contributor

mrahhal commented Oct 28, 2016

@davidfowl that would be super cool. Because with the current state, IApplicationLifetime.ApplicationStopping is kinda useless (and harmful even).

I have simple tasks that can run for a longish time. And even if an exception happens I have some fault tolerance built in. But in the current version the thread is being shutdown aprubtly that even finally blocks are not being executed. So that just renders the whole thing useless.

I have something like the following (assume that StartTask will run only once at a time):

public class SomeService
{
    private ILogger<SomeService> _logger;
    private IServiceProvider _provider;
    private Task _task;

    public SomeService(
        ILogger<SomeService> logger,
        IApplicationLifetime appLifetime,
        IServiceProvider provider)
    {
        _logger = logger;
        _provider = provider;

        appLifetime.ApplicationStopping.Register(() =>
        {
            try
            {
                _logger.LogInformation("Waiting for task.");
                _task?.Wait();
                _logger.LogInformation("Finished waiting for task.");
            }
            catch (Exception ex)
            {
                _logger.LogError("Error.", ex);
            }
        });
    }

    public void StartTask()
    {
        _task = StartTaskCoreAsync();
    }

    public async Task StartTaskCoreAsync()
    {
        _logger.LogInformation("Starting.");
        await Task.Delay(5000);
        _logger.LogInformation("Finished.");
    }
}

When I hit ctrl+c after I issue the request that calls this service, every single time the only logs I'm seeing are:
"Starting." "Waiting for task." And then "Press any key to continue..." (yikes)

Now, this is a simple example. But this is extremely harmful for things like a background processing jobs library that depends on ApplicationStopping to make sure jobs are persisted or whatever (which I'm also working on). And waiting for 1.1.0 is.. I don't know, not a good solution at all.

@davidfowl
Copy link
Member

@mrahhal since IApplicationLifetime is a service, you should be able to use the implementation we fixed in 1.1.0 without too many changes. Copy the code from https://github.com/aspnet/Hosting/blob/6627efecbfbe2dd007918b77e3c0311c1234d7d2/src/Microsoft.AspNetCore.Hosting/Internal/ApplicationLifetime.cs (probably change the name to avoid conflicts) into your project and register it as a service.

public void ConfigureServices(IServiceCollection services)
{
    services.AddSingleton<IApplicationLifetime>(new ApplicationLifetime());
}

@mrahhal
Copy link
Contributor

mrahhal commented Oct 28, 2016

That would have been a good solution, but here the web host is actually instantiating it and then using it here when it's being disposed. Ah hold on, this might actually work, will try it out.

@mrahhal
Copy link
Contributor

mrahhal commented Oct 28, 2016

@davidfowl so this did not work at all, as I thought. The WebHost is instantiating its own ApplicationLifetime and there's nothing I can do about it.

It's important to note that this issue makes all services/libraries depending on the documented behavior of IApplicationLifetime.ApplicationStopping completely faulty. It'll always fail if the registered action takes a bit more than a couple of hundred cycles to execute. Unfortunately, the problem will go unnoticed if your tasks were fast enough when you were actually looking.

Is a patch release a lot of work? I'm not sure if it happened before, but this is definitely worth it. Especially if 1.1.0 is more than a couple of days away.

@davidfowl
Copy link
Member

@mrahhal we're looking at putting this in a patch release. It's going to be proposed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants