Catch exceptions while disposing

[Tragetaschen]

Kestrel#9 revealed a shutdown problem when an exception is thrown during the final Dispose. This exception would omit setting the ManualResetEvent and the runtime would essentially deadlock.

This PR makes an exception during Dispose visible.

[davidfowl]

Dispose should never throw. We should fix Kestrel

[Tratcher]

Yes, we should fix Kestrel so it doesn't throw. That said, since Hosting will be interacting with 3rd party components here we should make it more defensive, especially when the consequence is a hang.

[Eilon]

Yeah what @Tratcher said. Need to fix both sides.

[Tragetaschen]

This isn't meant as a fix for the Kestrel bug.

The problem it tries to solve is that the Dispose exception cannot be noticed at the moment:

The RequestShutdown() calls the handler by Cancel'ing the CancellationTokenSource. An Exception during the Register'ed ShutdownRequested handler stops setting the ManualResetEvent and the program deadlocks. That's bad, so let's assume, there is at least a try/finally for the Set(). Now, when the CTS cancels, the Exceptions are thrown when all handlers ran. These exceptions unwind into the ignored Task and set this to the Faulted state. Currently, noone checks the ignored-Task's state, so the exceptions are propagated on the finalizer thread (ugh!). To safely check the exceptions, you would have to Wait() for the Task, otherwise, it's a race. But other parts of the program besides the ignored-Task can call RequestShutdown(). This means the ignored-Task might not run to an end and we are back to the deadlock.

I don't see it yet, but there sould be a better design here for exceptions during shutdown. Until then if a Dispose throws, this makes it possible to see the Exceptions at least.

[Tratcher]

We might want to use ILogger instead of going directly to the Console.

[Eilon]

Use Environment.NewLine instead of \n?

[davidfowl]

This error message formatting doesn't look right. There is an overload that takes an exception object.

[Tratcher]

Looks good.

[davidfowl]

[davidfowl]

@Tragetaschen has a CLA on file AFAIK /cc @Eilon I think this is good to merge

[Eilon]

Yup CLA is good.