-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
On Linux, failing to bind the address on Start generates a coredump #5920
Comments
I need to verify if generating the coredumps are default behavior on a Fedora system or whether this is something I configured myself (and forgot about). |
Interesting. Unfortunately, not throwing is a breaking change at this point. |
That try/catch would go in Program.Main, that's where the process lifetime is controlled. |
Until last week, I wasn't aware of the coredumps (and I guess probably you weren't either). On Fedora, coredumps are enabled by default. The OS also does cleanup to avoid filling the disk. |
That makes sense. Perhaps it's ok to catch any type of exception and return exitcode 1? Can you set a milestone? |
I'd be interested to know what other stacks do here. Having specific logic like this in the framework or the template seems wrong due to the (non-destructive) behavior of a single distro. It might be surprsing, but I'm not sure it warrants guarding against. |
Moving to discussions based on @DamianEdwards' last comment. |
I didn't find time yet to look at other stacks.
Many distros noop coredumps because their users don't know what to do with them, and they don' t support cleaning them up. That is the distro's choice. Our choice is whether we end up calling imo, we are not in such an abnormal case, the webserver can terminate normally with a non-zero exit code. |
As far as the runtime’s concerned a unhandled exception is an abnormal process termination. The runtime has always called abort or on Windows TerminateProcess in these cases. All the other distros and Windows don’t generate a core dump by default.
/cc: @jkotas
|
@mikem8361 I'm not sure how you read my comment. For me, the runtime behaves properly by calling
By
I meant: it is our choice to catch the exception or let it go to the runtime. |
Where do you suggest we catch the exception? Do you think that WebHost.Run() should just log and exit gracefully if the server fails to start? |
Yes, a method on |
When the server is already bound: |
public class Program
{
public static int Main(string[] args)
{
return CreateWebHostBuilder(args).Build().RunMain();
}
public static IWebHostBuilder CreateWebHostBuilder(string[] args) =>
WebHost.CreateDefaultBuilder(args)
.UseStartup<Startup>();
}
public static class WebHostExtensions
{
public static int RunMain(this IWebHost host)
{
return host.RunMainAsync().GetAwaiter().GetResult();
}
public static async Task<int> RunMainAsync(this IWebHost host, CancellationToken token = default)
{
try
{
await host.RunAsync(token);
return 0;
}
catch
{
// Exception is logged by host.
return 1;
}
}
} @halter73 this application doesn't terminate when the host throws a bind exception. backtrace looks like the main thread is waiting for the finalizer thread:
|
What is the finalizer thread doing when this happens? I expect that it is raising |
@jkotas you are right:
|
debugging with ASP.NET Core 2.0 (for which I have libsosplugin matching lldb) shows
|
The issue has already been fixed here: aspnet/Hosting#1432. It is not in 2.1. |
@DamianEdwards I've provided the info you requested (https://github.com/aspnet/Hosting/issues/1416#issuecomment-397990693): on other stacks the process terminates normally with a non-zero return code. |
@DamianEdwards @halter73 @muratg can you look at this and decide what to do about it? |
Yea this should be patched |
I agree we should do something @davidfowl, but what exactly do you suggest we patch? We don't change templates in patches, do we? I think we should not call abort for unhandled exception and just terminate with a non-zero exit code like ruby, node, python, java etc... |
Specifically the hang here https://github.com/aspnet/Hosting/issues/1416#issuecomment-400362648 |
@davidfowl The hang is fixed in 2.2 (and 3.0+). I don't think it would meet 2.1 bar at this point, but if you disagree, please file a specific bug for 2.1. Re: core-dump vs displaying a stack-trace and exiting with |
It doesn't look like there's any action on ASP.NET here. @davidfowl, if you disagree, please reopen. |
To address this, either a change is needed in the runtime or in ASP.NET Core. Proposing a change to ASP.NET Core: Change the generic host So: public static void Main(string[] args)
=> CreateWebHostBuilder(args).Build().Run(); becomes: public static int Main(string[] args)
=> CreateWebHostBuilder(args).Build().Run(); This When using this There should be a another method on the host ( Using the @Tratcher @davidfowl wdyt? |
You would have to plum that through a lot of layers to get down to the server that generated the original binding error. It would also leave us with overlapping error handling mechanics. I still don't see a reason to special case this class of errors from a framework perspective. failing to bind is fatal and if you don't want a dump for some fatal errors then you can handle those in Main. |
I'm thinking of a try catch for any exception that prints the exception and turns it into a That gives you the same behavior like other stacks (ruby, node, python). This pattern is also in the System.CommandLine package by default: https://github.com/dotnet/command-line-api/blob/750f2365800c8e894f4bc3e54bbc73dcc9623f1d/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs#L147. |
@davidfowl ping |
I like aligning our behavior with other stacks, but this proposal doesn't quite do that. dotnet/coreclr/issues/17929 more fully aligns behavior, right? I support doing that instead, since I don't think flowing error codes is idiomatic in C#. |
I agree with that. With the change in dotnet/coreclr#21300, the Host will need to deal with setting the ExitCode (#6526). This means the C# becomes anyhow aware of exit codes. So we can't fully push it to the runtime. This Run method would put everything in one place where we go from C# exceptions to process exit codes. |
We periodically close 'discussion' issues that have not been updated in a long period of time. We apologize if this causes any inconvenience. We ask that if you are still encountering an issue, please log a new issue with updated information and we will investigate. |
When starting Kestrel on a port that is taken an exception is thrown. e.g.:
Since this exception is not handled, the runtime calls
abort
which (normally) generates a coredump (see https://github.com/dotnet/coreclr/issues/17929).The behavior seems excessive for failing to bind the port. Maybe there should be a try/catch somewhere to turn this into a non-success process exit?
It may not be obvious these coredumps are being generated. And when seeing these dumps on their system, users may get the wrong impression dotnet failed in some bad way.
CC @davidfowl @halter73 @mikem8361
The text was updated successfully, but these errors were encountered: