When the libuv thread fails fail accepts on the listener

[davidfowl]

Now that we have a pull model for accepting connections and Kestrel owns the transport loop, we no longer have to shutdown when the libuv loop thread ends ungracefully. Instead, we should capture the exception and complete the channel that stored accepted connections with the exception. This removes the dependency on IHostApplicationLifetime and cleans up this code https://github.com/aspnet/AspNetCore/blob/a1e77a2c090e62bc3be3383c34d521449d34e74c/src/Servers/Kestrel/Transport.Libuv/src/Internal/LibuvThread.cs#L332-L334

[analogrelay]

I'm feeling super "meh" on the value of this. What does "clean up" mean in this context? What is the customer getting out of this?

[davidfowl]

The biggest benefit here is removing the dependency on the lifetime in the transport itself. It the consumer would be responsible to terminating the host if accept failed.

[RiskRunner0]

@davidfowl for this, do we just need to get rid of the _appLifetime references? If so, I can put out a PR for this

[davidfowl]

The work here is the following:

• Remove the dependency on IApplicationLifetime
• Complete all accept queues bound to that specific LibuvThread (https://github.com/aspnet/AspNetCore/blob/fd060ce8c36ffe195b9e9a69a1bbd8fb53cc6d7c/src/Servers/Kestrel/Transport.Libuv/src/Internal/ListenerContext.cs#L161) with this exception (https://github.com/aspnet/AspNetCore/blob/fd060ce8c36ffe195b9e9a69a1bbd8fb53cc6d7c/src/Servers/Kestrel/Transport.Libuv/src/Internal/LibuvThread.cs#L331)

[RiskRunner0]

Cool, will do the work and send out the PR, thanks for the clarification.

[RiskRunner0]

Sorry, relatively new and still trying to understand this.. I cleaned up the IApplicationLifetime, but I'm ont really clear on what you mean in bullet 2. Can you elaborate for me?

[davidfowl]

The LibuvConnectionListener creates a set of threads that run the event loop (the LibuvThread class) https://github.com/aspnet/AspNetCore/blob/04bf1bf32e13d35d4d7f61f9cbeb9bfa22e0c617/src/Servers/Kestrel/Transport.Libuv/src/Internal/LibuvConnectionListener.cs#L134-L142.

Those threads are then associated with listeners here https://github.com/aspnet/AspNetCore/blob/04bf1bf32e13d35d4d7f61f9cbeb9bfa22e0c617/src/Servers/Kestrel/Transport.Libuv/src/Internal/LibuvConnectionListener.cs#L144-L169. There are 3 kinds of listeners:

• Listener
• ListenerPrimary
• ListenerSecondary

All operations happen on loop threads (all IO operations, reads, writes, accepts etc) and if a thread stops running it means nothing will work. The current code would try to stop the host from any of the running threads if it ended in an error. This is because the contract between the transport and the server (kestrel in this case) was push based (the transport would call into kestrel when there was a new connection and there was no OnError callback per se).

In the new model, Kestrel is polling AcceptAsync on the transport and we want to make sure it throws the exception when a thread stops. That means we need to complete the acceptQueue in the appropriate listeners when the thread dies.

[RiskRunner0]

Hey @davidfowl sorry for the delay, life stuff came up. How does the exception come in to play for this?

[vaughanr]

Hi @davidfowl, I've attempted to fix this issue with a pr. I'm very happy to change it if this isn't what you had in mind. Thanks.

[davidfowl]

Closing this out, not worth the pain