Allow ConnectionListener.AcceptAsync() to return null

[halter73]

Background and Motivation

When migrating Kestrel to the new System.Net.Connections APIs, I noticed that listeners need to throw from AcceptAsync when shutting down since AcceptAsync cannot return null (at least according to the signature).

In the case where there are ongoing AcceptAsync calls when the listener is disposed, I would prefer to be able to return null rather than throw an exception in order to avoid unnecessary first-chance exceptions when listeners are gracefully shutting down. For listeners based on managed Sockets, there's likely to be an exception either way, but other listeners my be able to avoid throwing exceptions during shutdown entirely.

null should never be returned unless the listener has been closed. null indicates to accept loops that they should exit.

Proposed API

namespace System.Net.Connections
{
    public class ConnectionListener
    {
-        public abstract System.Threading.Tasks.ValueTask<System.Net.Connections.Connection> AcceptAsync(System.Net.Connections.IConnectionProperties? options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken));
+        public abstract System.Threading.Tasks.ValueTask<System.Net.Connections.Connection?> AcceptAsync(System.Net.Connections.IConnectionProperties? options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken));
    }

Alternative Designs

Throwing an OperationCanceledException, ObjectDisposedException or similar instead of returning null.

cc @geoffkizer @scalablecory @davidfowl

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

[davidfowl]

I though we agreed to support returning null

[geoffkizer]

In the case where there are ongoing AcceptAsync calls when the listener is disposed, I would prefer to be able to return null rather than throw an exception

It's worth pointing out that it's almost always the case that there's an outstanding AcceptAsync when the listener is disposed. In other words, this isn't some corner case, it's the common case, and that's why we want to avoid exceptions here.

[terrajobst]

Video

• Looks good as proposed

namespace System.Net.Connections
{
    public partial class ConnectionListener
    {
        public abstract ValueTask<Connection?> AcceptAsync(
            IConnectionProperties? options = null,
            CancellationToken cancellationToken = default);
    }
}

[karelz]

Reopening for 5.0 backport in #41453

[karelz]

Fixed in 6.0/master (PR #41442) and in 5.0 (PR #41453).