Exception in Dispose() prevents host from shutting down

[meum]

I'm not sure if this is a bug or not, but I could not find it documented anywhere.
When a service which implements IDisposable throws an exception in Dispose(), the host never shuts down.

To reproduce, run this code and hit Ctrl + C

    public class Program
    {
        public static void Main(string[] args)
        {
            try
            {
                IHost host = Host.CreateDefaultBuilder(args)
                    .ConfigureServices(services => services.AddSingleton<Class>())
                    .Build();

                host.Services.GetRequiredService<Class>();
                host.Run();
            }
            catch (Exception ex)
            {
                Console.WriteLine(ex);
            }
        }
    }

    public class Class : IDisposable
    {
        public void Dispose()
        {
            throw new Exception();
        }
    }

After a few seconds. this is printed:

info: Microsoft.Hosting.Lifetime[0]
      Waiting for the host to be disposed. Ensure all 'IHost' instances are wrapped in 'using' blocks.

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

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

Issue Details

---

I'm not sure if this is a bug or not, but I could find it documented anywhere.
When a service which implements IDisposable throws an exception in Dispose(), the host never shuts down.

To reproduce, run this code and hit Ctrl + C

    public class Program
    {
        public static void Main(string[] args)
        {
            try
            {
                IHost host = Host.CreateDefaultBuilder(args)
                    .ConfigureServices(services => services.AddSingleton<Class>())
                    .Build();

                host.Services.GetRequiredService<Class>();
                host.Run();
            }
            catch (Exception ex)
            {
                Console.WriteLine(ex);
            }
        }
    }

    public class Class : IDisposable
    {
        public void Dispose()
        {
            throw new Exception();
        }
    }

After a few seconds. this is printed:

info: Microsoft.Hosting.Lifetime[0]
      Waiting for the host to be disposed. Ensure all 'IHost' instances are wrapped in 'using' blocks.

Author:	meum
Assignees:	-
Labels:	area-Extensions-Hosting, untriaged
Milestone:	-

[eerhardt]

Looks like the issue here is that the ConsoleLifetime service isn't getting disposed, and thus hanging the process:

runtime/src/libraries/Microsoft.Extensions.Hosting/src/Internal/ConsoleLifetime.cs

Lines 79 to 90 in 65bcece

	private void OnProcessExit(object sender, EventArgs e)
	{
	ApplicationLifetime.StopApplication();
	if (!_shutdownBlock.WaitOne(HostOptions.ShutdownTimeout))
	{
	Logger.LogInformation("Waiting for the host to be disposed. Ensure all 'IHost' instances are wrapped in 'using' blocks.");
	}
	_shutdownBlock.WaitOne();
	// On Linux if the shutdown is triggered by SIGTERM then that's signaled with the 143 exit code.
	// Suppress that since we shut down gracefully. https://github.com/dotnet/aspnetcore/issues/6526
	System.Environment.ExitCode = 0;
	}

runtime/src/libraries/Microsoft.Extensions.Hosting/src/Internal/ConsoleLifetime.cs

Lines 104 to 106 in 65bcece

	public void Dispose()
	{
	_shutdownBlock.Set();

The reason ConsoleLifetime isn't getting disposed is because the exception in the Class service is being thrown first, so none of the other services get disposed.

The following DI code captures the first exception from a service and returns it without disposes the rest of the services:

runtime/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ServiceProviderEngineScope.cs

Lines 106 to 132 in 65bcece

	try
	{
	for (int i = toDispose.Count - 1; i >= 0; i--)
	{
	object disposable = toDispose[i];
	if (disposable is IAsyncDisposable asyncDisposable)
	{
	ValueTask vt = asyncDisposable.DisposeAsync();
	if (!vt.IsCompletedSuccessfully)
	{
	return Await(i, vt, toDispose);
	}
	// If its a IValueTaskSource backed ValueTask,
	// inform it its result has been read so it can reset
	vt.GetAwaiter().GetResult();
	}
	else
	{
	((IDisposable)disposable).Dispose();
	}
	}
	}
	catch (Exception ex)
	{
	return new ValueTask(Task.FromException(ex));
	}

[davidfowl]

Isn't throwing from dispose a no-no? (https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1065) and

https://docs.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose

To help ensure that resources are always cleaned up appropriately, a Dispose method should be idempotent, such that it is callable multiple times without throwing an exception. Furthermore, subsequent invocations of Dispose should do nothing.

I don't think we should do anything here (wrt to disposable in DI).

As for the hang, that is a dupe of #50397 (sorta). One thing we can do is un-unify the shutdown paths between process exit and CLTR +C/SIGINT.

If CTRL+C/SIGINT fires, we can disable blocking process exit until dispose. That logic is fraught with problems anyways but we can't handle the SIGTERM case on non-windows without this #50527.