Skip to content
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

Deadlock when calling System.Environment.Exit(int exitCode) during Startup.Configure(..) #50397

Closed
armingerten opened this issue Jul 6, 2018 · 17 comments · Fixed by #56057
Closed
Assignees
Milestone

Comments

@armingerten
Copy link

In some situations my web application requires to immediately quit during start-up. When doing so via System.Environment.Exit(int exitCode), the dotnet process does not exit.

Minimal set-up to reproduce:

public static void Main(string[] args)
{
    var host = new WebHostBuilder()
        .UseKestrel()
        .UseStartup<Startup>()
        .Build();

    host.Run();
}
public override void Configure(IApplicationBuilder app)
{
	System.Environment.Exit(1);
}

It seems that in this scenario Set() will no longer be called on the instance of ManualResetEventSlim passed to AttachCtrlcSigtermShutdown(..) resulting in the Shutdown() method to block indefinitely.

https://github.com/aspnet/Hosting/blob/97c9510a95e676324ed8fa03d8b6dcf3b1c19ccf/src/Microsoft.AspNetCore.Hosting/WebHostExtensions.cs#L134-L162

One possible solution could be applying the WebHostOptions.ShutdownTimeout when waiting for the manual reset event.

System Info:

  • dotnet core: 2.1
  • Microsoft.AspNetCore.Hosting: 2.1.1
@Tratcher
Copy link
Member

Tratcher commented Jul 6, 2018

Are you using 2.0 or 2.1?

Have you considered throwing an exception from Startup.Configure rather than calling exit?

@armingerten
Copy link
Author

Thanks for the quick reply!

I am using 2.1.

I considered throwing exceptions from Startup.Configure, but I am using a JSON-based log-format and would like to keep the stacktrace in one log-entry (rather than plain-text, line-separated).

@muratg
Copy link

muratg commented Aug 28, 2018

@Tratcher a bug?

@Tratcher
Copy link
Member

Yes, with a workaround.

@aspnet-hello aspnet-hello transferred this issue from aspnet/Hosting Dec 18, 2018
@b0wter
Copy link

b0wter commented Dec 21, 2018

I am encountering the same behaviour. When throwing an exception instead it results in the same behaviour. As strack trace is written and I am stuck at

Application is shutting down...

I am running 2.1 on Linux (Fedora).

@alex-sherman
Copy link

I encounter the same behavior in 2.2. I had relied on Environment.Exit in 2.0 on Linux/Windows without problem, but now in 2.2 it hangs on both Windows and Linux. Attached is a debugger which I believe shows the deadlock in the console logger.

image

@dadude999
Copy link

I see the same issue on Windows. One workaround appears to be simply P/Invoking the native C exit function in a class somewhere as follows:

private const string NativeCLibName = "ucrtbase.dll"; // assumes Windows; on Mac it will be "libSystem.dylib", on Linux probably "libc.so"

private const string ExitFunctionNativeName = "exit";

[DllImport(NativeCLibName, EntryPoint = ExitFunctionNativeName)]
public static extern void NativeExit(int exitCode);

Then wherever you would have called Environment.Exit, just call NativeExit instead.

@davidfowl
Copy link
Member

If we can reproduce this with the generic host then we can move this to dotnet/runtime. It might require changes there anyways.

@davidfowl davidfowl self-assigned this Mar 28, 2021
@ghost
Copy link

ghost commented Mar 29, 2021

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@davidfowl
Copy link
Member

This isn't something we want to fix in the webhost, but we should fix this in the generic host.

@davidfowl davidfowl transferred this issue from dotnet/aspnetcore Mar 30, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-Extensions-Hosting untriaged New issue has not been triaged by the area owner labels Mar 30, 2021
@ghost
Copy link

ghost commented Mar 30, 2021

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

In some situations my web application requires to immediately quit during start-up. When doing so via System.Environment.Exit(int exitCode), the dotnet process does not exit.

Minimal set-up to reproduce:

public static void Main(string[] args)
{
    var host = new WebHostBuilder()
        .UseKestrel()
        .UseStartup<Startup>()
        .Build();

    host.Run();
}
public override void Configure(IApplicationBuilder app)
{
	System.Environment.Exit(1);
}

It seems that in this scenario Set() will no longer be called on the instance of ManualResetEventSlim passed to AttachCtrlcSigtermShutdown(..) resulting in the Shutdown() method to block indefinitely.

https://github.com/aspnet/Hosting/blob/97c9510a95e676324ed8fa03d8b6dcf3b1c19ccf/src/Microsoft.AspNetCore.Hosting/WebHostExtensions.cs#L134-L162

One possible solution could be applying the WebHostOptions.ShutdownTimeout when waiting for the manual reset event.

System Info:

  • dotnet core: 2.1
  • Microsoft.AspNetCore.Hosting: 2.1.1
Author: armingerten
Assignees: davidfowl
Labels:

area-Extensions-Hosting, untriaged

Milestone: -

@ghost ghost added this to Untriaged in ML, Extensions, Globalization, etc, POD. Mar 30, 2021
@davidfowl
Copy link
Member

Here's the repro with the generic host:

using System;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;

var host = Host.CreateDefaultBuilder()
        .ConfigureServices(services =>
        {
            services.AddHostedService<Worker>();
        })
        .Build();

host.Run();

class Worker : IHostedService
{
    public Task StartAsync(CancellationToken cancellationToken)
    {
        Environment.Exit(1);
        return Task.CompletedTask;
    }

    public Task StopAsync(CancellationToken cancellationToken)
    {
        return Task.CompletedTask;
    }
}

Results in hanging at this:

image

@davidfowl
Copy link
Member

@janvorli All of hosting weird designs are around trying to let the program continue execution before sigterm tears the application down (blocking on a wait handle in the event and waiting for the application code to signal it when it's done). Is there anyway we can stop doing this? Any cleaner way the runtime can support this natively without us doing these tricks?

@maryamariyan maryamariyan removed the untriaged New issue has not been triaged by the area owner label Mar 30, 2021
@maryamariyan maryamariyan added this to the 6.0.0 milestone Mar 30, 2021
@ghost ghost moved this from Untriaged to 6.0.0 in ML, Extensions, Globalization, etc, POD. Mar 30, 2021
@janvorli
Copy link
Member

@davidfowl I am not sure if I fully understand what tricks you are talking about, so let me answer a bit broadly. We've been trying to get rid of all attempts to synchronize with process exiting / cleaning stuff up in the runtime during the past years, as they've often lead to intermittent, sometimes very rare and hard to debug deadlocks during process exit. Or process shutdown delays. On Unix, after the main thread exits, the secondary threads can still be running code (including managed code) for some time until the OS shuts down the process.

As for SIGTERM handling in the runtime, we cannot execute the termination code in the SIGTERM handler directly, as only so called async safe functions can be called from it. The reason is that the signal can be delivered to any thread of the process any time. So for example, if the thread that the signal handler is called on was inside of malloc code and the handler then attempted to allocate memory too, it could lead to deadlock or memory corruption depending on where in the malloc code the thread was running when it was interrupted for the signal. That's why we run it on a special thread and the signal handler just notifies that thread.

@davidfowl
Copy link
Member

davidfowl commented Mar 31, 2021

Right. What we're trying to do is let the main thread unwind. Many systems use SIGTERM as a way to start a graceful shutdown. To make this work we need to start the shutdown process and wait for some timeframe until it's done. To coordinate that, we set up a bunch of events to allow the main thread to exit before continuing the process exit handler.

Ideally we could have the runtime coordinate this so that we can remove our code (similar to how Console.CancelKeyPress allows cancelling shutdown). Here's a narrowed down piece of code:

using System;
using System.Threading;

namespace ConsoleApp35
{
    class Program
    {
        static void Main(string[] args)
        {
            var waitForProcessShutdownStart = new ManualResetEventSlim();
            var waitForMainExit = new ManualResetEventSlim();

            AppDomain.CurrentDomain.ProcessExit += (sender, e) =>
            {
                // Tell everyone that shutdown has started
                waitForProcessShutdownStart.Set();

                Console.WriteLine("Waiting for main");
                // Don't unwind until main exists
                waitForMainExit.Wait();
            };

            Console.CancelKeyPress += (sender, e) =>
            {
                e.Cancel = true;

                // Tell everyone that shutdown has started
                waitForProcessShutdownStart.Set();
            };

            Console.WriteLine("Waiting for shutdown");
            // Wait for shutdown to start
            waitForProcessShutdownStart.Wait();

            // This is where we do graceful shutdown

            // This will cause the hang because it invokes the process exit handler directly
            // Environment.Exit(1);

            // Now we're done with main, tell the shutdown handler
            waitForMainExit.Set();
        }
    }
}

@davidfowl
Copy link
Member

To makes matters more complex, Environment.Exit, synchronously triggering the OnProcessExit callback. In the above scenario this causes a deadlock. The most pragmatic thing we could do here is to timeout the wait so it isn't a permanent deadlock...

cc @maryamariyan

@davidfowl
Copy link
Member

Closing this in favor of #50527

ML, Extensions, Globalization, etc, POD. automation moved this from 6.0.0 to Done Mar 31, 2021
@ghost ghost locked as resolved and limited conversation to collaborators May 1, 2021
eerhardt added a commit to eerhardt/runtime that referenced this issue Jul 20, 2021
Don't listen to ProcessExit on net6.0+ in Hosting anymore. This allows for Environment.Exit to not hang the app.
Don't clobber ExitCode during ProcessExit now that SIGTERM is handled separately.

For non-net6.0 targets, only wait for the shutdown timeout, so the process doesn't hang forever.

Fix dotnet#55417
Fix dotnet#44086
Fix dotnet#50397
Fix dotnet#42224
Fix dotnet#35990
eerhardt added a commit that referenced this issue Jul 22, 2021
* Add NetCoreAppCurrent target to Microsoft.Extensions.Hosting

* Handle SIGTERM in Hosting and handle just like SIGINT (CTRL+C)

Don't listen to ProcessExit on net6.0+ in Hosting anymore. This allows for Environment.Exit to not hang the app.
Don't clobber ExitCode during ProcessExit now that SIGTERM is handled separately.

For non-net6.0 targets, only wait for the shutdown timeout, so the process doesn't hang forever.

Fix #55417
Fix #44086
Fix #50397
Fix #42224
Fix #35990

* Remove _shutdownBlock on netcoreappcurrent, as this is no longer waited on
* Change Console.CancelKeyPress to use PosixSignalRegistration SIGINT and SIGQUIT
* Use a no-op lifetime on mobile platforms

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

Successfully merging a pull request may close this issue.

9 participants