dotnet/runtime

Connection Abstractions

scalablecory opened this issue · 73 comments

System.Net.Connections

Connections is an abstraction for composable connection establishment. It aims to improve layering separation and provide a standard extensibility model for making network connections.

Connections targets client/server implementations, and their users with advanced needs to plug in custom functionality. The latter is a heavily requested feature for HttpClient, and the Kestrel team has a handful of examples of users taking advantage of this pattern.

Connections brings .NET into parity with “modern” transport models such as Go’s dialers and Netty's channels. ASP.NET has a similar set of interfaces (“Bedrock Transports”) that this would supersede.

Basic API usage

At its most basic usage, System.Net.Connections is an abstraction over Socket.Accept and Socket.Connect. For this Socket usage today:

// server
using var listener = new Socket(SocketType.Stream, ProtocolType.Tcp);
listener.Bind(new IPEndPoint(IPAddress.IPv6Loopback, 0));
listener.Listen();
using Socket connection = await listener.AcceptAsync();
using Stream connectionStream = new NetworkStream(connection);

// client
using var socket = new Socket(SocketType.Stream, ProtocolType.Tcp);
await socket.ConnectAsync(new DnsEndPoint("contoso.com", 80));
using var stream = new NetworkStream(socket);

The equivalent with System.Net.Connections is:

// server
using IConnectionFactory factory = new SocketsConnectionFactory(SocketType.Stream, ProtocolType.Tcp);
using IConnectionListener listener = await factory.BindAsync(new IPEndPoint(IPAddress.IPv6Loopback, 0));
using IConnection connection = await listener.AcceptAsync();
using Stream stream = connection.Stream;

// client
using IConnectionFactory factory = new SocketsConnectionFactory(SocketType.Stream, ProtocolType.Tcp);
using IConnection connection = await factory.ConnectAsync(new DnsEndPoint("contoso.com", 80));
using Stream stream = connection.Stream;

Composability

Composability has been modeled after Stream. For instance:

Stream stream;
stream = new NetworkStream(new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp));
stream = new BufferedStream(stream);
stream = new GZipStream(stream, CompressionLevel.Optimal);

While Streams compose the raw byte stream, Connections compose the establishment of that stream:

// Create a connection factory.
IConnectionFactory factory;
factory = new SocketsConnectionFactory(SocketType.Stream, ProtocolType.Tcp);
factory = new SslConnectionFactory(factory);

// Establish a connection.
IConnection connection = await factory.ConnectAsync(endPoint, options);
Stream stream = connection.Stream;

Beyond things like TLS and sockets, library implementation can separate some connection establishment logic into clean layers, as seen in HttpClient here:

// Setup connection factory base. Either use the user's custom connection factory, or sockets.
_tcpConnectionFactory =
    settings._connectionFactory != null
    ? (IConnectionFactory)new EatDisposeConnectionFactory(settings._connectionFactory)
    : new SocketsConnectionFactory(SocketType.Stream, ProtocolType.Tcp);

// Middleware that selects which endpoint to connect to, routing through proxies.
_tcpConnectionFactory = new TransportSelectionMiddleware(_tcpConnectionFactory);

// Middleware to setup TLS. If the user's custom connection factory already sets up TLS, it's a no-op. Otherwise, it does it for them.
_tcpConnectionFactory = new HttpsConnectionMiddleware(_tcpConnectionFactory);

Extensibility

Components can expose a connection factory to the user as an extension model. As an example, a user might implement a bandwidth monitoring extension for HttpClient:

IConnectionFactory factory;
factory = SocketsHttpHandler.CreateConnectionFactory();
factory = new BandwidthMonitoringMiddleware(factory);

SocketsHttpHandler handler = new SocketsHttpHandler();
handler.SetConnectionFactory(factory);

Requests for HttpClient extensibility

As an example of how users would make use of this, we’ve seen many requests for extensibility in HttpClient:

Go example

Here we use a SOCKS proxy with Go's HTTP client, via a dialer:

auth := proxy.Auth{
    User:     "YOUR_PROXY_LOGIN",
    Password: "YOUR_PROXY_PASSWORD",
}

dialer, err := proxy.SOCKS5("tcp", "contoso.com:12345", &auth, proxy.Direct)
tr := &http.Transport{Dial: dialer.Dial}
myClient := &http.Client{
    Transport: tr,
}

Netty example

And use Netty's channel pipelines to do the same:

ChannelPipeline p = ch.pipeline();
p.addFirst(new Socks5ProxyHandler(new InetSocketAddress("contoso.com", 12345), "YOUR_PROXY_LOGIN", "YOUR_PROXY_PASSWORD"));
p.addLast(new HttpClientCodec());

Connection Properties

Streams have implementation-specific functionality and properties. For instance, Socket.Shutdown() and various properties on SslStream. This means that, even when composing them, the user must still keep track of multiple layers:

using Socket socket = new Socket(SocketType.Stream, ProtocolType.Tcp);
using Stream networkStream = new NetworkStream(socket);
using SslStream sslStream = new SslStream(networkStream);
using StreamWriter writer = new StreamWriter(sslStream) { AutoFlush = true };

Console.WriteLine($"Connected via TLS: {sslStream.CipherAlgorithm} {sslStream.CipherStrength}");
writer.Write("Hello World.");
socket.Shutdown(SocketShutdown.Send);

This makes it challenging to build extensibility into a library's design:

  • One can no longer just ask a user for a Stream, but instead every piece of layer that they need to override.
  • Multiple callbacks may be needed to hide certain layers from the user, to allow them to implement only exactly what they need.

With connections, this is cleaned up by allowing each layer in a composed connection to expose, override, or hide features from the previous layer. Abstractions can be used to avoid exposing specific implementation.

Here, our TLS implementation exposes a property while passing through unknown types to previous layers.

bool IConnectionProperties.TryGet(Type propertyKey, [NotNullWhen(true)] out object? property)
{
    if ((propertyKey == typeof(ISslConnectionInformation))
    {
        property = (TProperty)(object)...;
        return true;
    }

    return _baseConnection.ConnectionProperties.TryGet(propertyKey, out property);
}

The type keys available in properties are not discoverable :( documentation must be read.

When using an established connection, we then extract it as well as a Socket property which was exposed by a previous sockets layer:

using IConnection connection = ...;
using StreamWriter writer = new StreamWriter(connection.Stream) { AutoFlush = true };

if (connection.ConnectionProperties.TryGet(out ISslConnectionProperties? sslProperties))
{
    Console.WriteLine($"Connected via TLS: {sslProperties.CipherAlgorithm} {sslProperties.CipherStrength}");
}

writer.Write("Hello World.");

if (connection.ConnectionProperties.TryGet(out Socket? socket))
{
    socket.Shutdown(SocketShutdown.Send);
}

Connection Establishment properties

Property bags are also used when establishing new connections, to allow each layer to decide how to establish the connection. For instance, HttpClient can implement a factory that tests if HTTPS is being used and inject TLS into a connection:

async ValueTask<IConnection> ConnectAsync(EndPoint endPoint, IConnectionProperties options, CancellationToken cancellationToken = default)
{
    if (!options.TryGet(out IHttpInternalConnectInfo httpProperties))
    {
        throw new HttpRequestException($"{nameof(HttpsConnectionMiddleware)} requires the {nameof(InternalConnectionInfoProperty)} connection property.");
    }

    HttpConnectionKind kind = httpProperties.Pool.Kind;

    if(kind == HttpConnectionKind.Https || kind == HttpConnectionKind.SslProxyTunnel)
    {
        return await _tlsConnectionFactory.ConnectAsync(endPoint, options, cancellationToken);
    }
    else
    {
        return await _plaintextConnectionFactory.ConnectAsync(endPoint, options, cancellationToken);
    }
}

Usage Examples

Establish a new connection and send data

async Task Send(IConnectionFactory factory)
{
    await using IConnection connection = await factory.ConnectAsync(new DnsEndPoint("contoso.com", 80));
    await using Stream s = connection.Stream;
    await using var sw = new StreamWriter(s);

    await sw.WriteAsync("GET / HTTP/1.1\r\n\r\n");
}

Listen for a new connection and receive data

async Task Receive(IConnectionListenerFactory factory)
{
    await using IConnectionListener listener = await factory.BindAsync(new IPEndPoint(IPAddress.Loopback, 0));
    await using IConnection connection = await listener.AcceptAsync();
    await using Stream s = connection.Stream;
    using var sr = new StreamReader(s);

    string requestLine = await sr.ReadLineAsync();
}

Proposed API

interface IConnectionProperties
{
    bool TryGet(Type propertyKey, [NotNullWhen(true)] out object? property);
}

// "easy mode" implementation for users; library implementors can write a custom one that merges allocation of IConnection, IConnectionProperties, and their properties.
public sealed partial class ConnectionPropertyCollection : IConnectionProperties
{
    public void Add<T>(T property);
    public bool TryGet(Type propertyKey, [NotNullWhen(true)] out object? property);
}

// this is separate from IConnection due to anticipation of QUIC. See QUIC straw man below.
interface IConnectionStream : IAsyncDisposable, IDisposable
{
    IConnectionProperties ConnectionProperties { get; }

    // If only one is implemented, the other should wrap. To prevent usage errors, whichever is retrieved first, the other one should throw.
    Stream Stream { get; }
    IDuplexPipe Pipe { get; }
}

interface IConnection : IConnectionStream
{
    EndPoint? LocalEndPoint { get; }
    EndPoint? RemoteEndPoint { get; }
}

// This could be split into two interfaces, one which has Connect and the other which has Bind. This would harm composability, but would avoid users needing to throw NotImplementedException if they only care about server-side.
interface IConnectionFactory : IAsyncDisposable, IDisposable
{
    ValueTask<IConnection> ConnectAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);
    ValueTask<IConnectionListener> BindAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);
}

interface IConnectionListener : IAsyncDisposable, IDisposable
{
    IConnectionProperties ListenerProperties { get; }
    EndPoint? LocalEndPoint { get; }
    ValueTask<IConnection> AcceptAsync(IConnectionProperties? options = null, CancellationToken cancellationToken = default);
}

static class ConnectionExtensions
{
    // Injects a simple stream filter without all the other ceremony.
    public static IConnectionFactory Filter(this IConnectionFactory factory, Func<IConnection, IConnectionProperties?, CancellationToken, ValueTask<Stream>> filter);
    public static IConnectionFactory Filter(this IConnectionFactory factory, Func<IConnection, IConnectionProperties?, CancellationToken, ValueTask<IConnection>> filter);

    // Generic wrapper for non-generic IConnectionProperties method.
    public static bool TryGet<T>(this IConnectionProperties properties, [MaybeNullWhen(false)] out T property);
}

Some thoughts:

  • A purer version of the API could move EndPoint parameters/properties into the IConnectionProperties.

Additional APIs

This integrates the above interfaces with HTTP, Sockets, TLS

namespace System.Net.Connections
{
    // needs documentation: exposes typeof(Socket) property in its connections.
    class SocketsConnectionFactory : IConnectionFactory
    {
        // dual-mode IPv6 socket. See Socket(SocketType socketType, ProtocolType protocolType)
        public SocketsConnectionFactory(SocketType socketType, ProtocolType protocolType);

        // See Socket(AddressFamily addressFamily, SocketType socketType, ProtocolType protocolType)
        public SocketsConnectionFactory(AddressFamily addressFamily, SocketType socketType, ProtocolType protocolType);

        public ValueTask<IConnectionListener> BindAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);
        public ValueTask<IConnection> ConnectAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);

        public void Dispose();
        protected virtual void Dispose(bool disposing);
        public virtual ValueTask DisposeAsync();

        // These exist to provide an easy way for users to override default behavior.
        // A more idiomatic (but more API-heavy) way to do this would be to pass some sort of ISocketConfiguration that has all the pre-connect socket options one could want.
        protected virtual Socket CreateSocket(SocketType socketType, ProtocolType protocolType, EndPoint? endPoint, IConnectionProperties? options);
        protected virtual Stream CreateStream(Socket socket, IConnectionProperties? options);
        protected virtual IDuplexPipe CreatePipe(Socket socket, IConnectionProperties? options);
    }

    [System.CLSCompliantAttribute(false)] // due to TlsCipherSuite
    interface ISslConnectionProperties
    {
        CipherAlgorithmType CipherAlgorithm { get; }
        int CipherStrength { get; }
        HashAlgorithmType HashAlgorithm { get; }
        int HashStrength { get; }
        ExchangeAlgorithmType KeyExchangeAlgorithm { get; }
        int KeyExchangeStrength { get; }
        X509Certificate LocalCertificate { get; }
        SslApplicationProtocol NegotiatedApplicationProtocol { get; }
        TlsCipherSuite NegotiatedCipherSuite { get; }
        X509Certificate RemoteCertificate { get; }
        SslProtocols SslProtocol { get; }
        TransportContext TransportContext { get; }
    }

    // needs documentation: exposes typeof(ISslConnectionProperties) property in its connections.
    // needs documentation: requires SslClientAuthenticationOptions and SslServerAuthenticationOptions options in connect/bind.
    class SslConnectionFactory : IConnectionFactory
    {
        public SslConnectionFactory(IConnectionFactory baseFactory);

        public void Dispose();
        protected virtual void Dispose(bool disposing);
        public virtual ValueTask DisposeAsync();

        public ValueTask<IConnectionListener> BindAsync(EndPoint endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);
        public ValueTask<IConnection> ConnectAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);
    }
}

namespace System.Net.Http
{
    sealed class SocketsHttpHandler
    {
        public static IConnectionFactory CreateConnectionFactory();
        public static IConnectionFactory CreateConnectionFactory(Func<HttpRequestMessage, DnsEndPoint, IConnectionProperties, Socket> createSocket);

        // Return a stream that isn't Socket-based.
        public static IConnectionFactory CreateConnectionFactory(Func<HttpRequestMessage, DnsEndPoint, IConnectionProperties, CancellationToken, ValueTask<Stream>> establishConnection);
        public static IConnectionFactory CreateConnectionFactory(Func<HttpRequestMessage, DnsEndPoint, IConnectionProperties, CancellationToken, ValueTask<IConnection>> establishConnection);

        // For users of the above two APIs, they can call this if they just want to wrap the defaults.
        public static Socket DefaultCreateSocket(HttpRequestMessage message, DnsEndPoint endPoint, IConnectionProperties options);
        public static ValueTask<IConnection> DefaultEstablishConnection(HttpRequestMessage message, DnsEndPoint endPoint, IConnectionProperties options, CancellationToken cancellationToken);

        public void SetConnectionFactory(IConnectionFactory factory);

        // Sets a pre-encryption filter.
        public void SetConnectionFilter(Func<HttpRequestMessage, DnsEndPoint, IConnection, CancellationToken, ValueTask<Stream>> filter);
        public void SetConnectionFilter(Func<HttpRequestMessage, DnsEndPoint, IConnection, CancellationToken, ValueTask<IConnection>> filter);
    }
}

Thoughts and Questions

  • Heavy use of property bags significantly harms discoverability: users need to read documentation to understand what features to expect from each layer.
    • This is not a common design pattern in corefx, but our ASP.NET users will be very familiar with DI and should have an easy enough time grasping it.
    • This puts additional burden on maintainers to ensure thorough documentation of required/exposed properties.
  • Shuffling features between layers with property bags may harm maintainability if authors need to pay attention to which layer introduces which property and how they all mold together.
    • To prove out this API, I implemented it feature-complete for HttpClient and did not observe this complexity. I believe there are not a large number of use cases one would want to introduce layers for, and so layering is unlikely to become complex enough for this to be a problem in most apps.
  • This API has a very large surface area. Are there enough benefits versus a simpler one (e.g. a Func<(string host, int port), Stream>)?
    • HttpClient has a strong need for a connection establishment abstraction. All but one need could be solved by the simpler one.
    • ... however, HTTP/3 and QUIC is significantly more involved and will need some more complex dialer abstraction anyway. Having some symmetry between the TCP and QUIC dialers may be beneficial.
    • This being the standard/recommended method to abstract connections across BCL, ASP.NET Core, and 3rd party libraries provides value over some simple library-specific callbacks.
  • How much impact will this have? How many 3rd party libraries are in need of this and would use it if available?
    • Not many people need this, but when they do it is really important to them. See HttpClient extensibility asks.
    • There are not many 3rd party libraries opening network connections.
  • Should we apply the extensibility model to other things? Does it make sense for e.g. SmtpClient, SqlConnection, and so on to make use of this?
    • For now, we have decided to be conservative and wait to see how this gets used before introducing those extensibility points.

Future QUIC amendments

QUIC is not yet out of draft status, so QUIC-specific APIs were not a focus for this proposal. However, current knowledge of QUIC did help shape the APIs to make adapting it easier. Here are some thoughts based on current experience:

  • QUIC has features that an app is required to be aware of. It is not possible for an app to write a generic protocol that is seamlessly usable between both TCP and QUIC without at least some small shim that knows how to do the QUIC-specific stuff:
    • There is no "keep sending in background after socket/process closed" concept, so some form of flushing of send buffers is required.
    • Streams are aborted with status codes, and can abort read and write side of a stream separately.
    • Connections are always closed with an status code.
    • There are no predefined codes to indicate success/failure: they are all application-specific. It's not clear what an abortive QuicStream.Dispose should do, for instance.
    • Protocols make use of unidirectional/bidirectional stream differentiation, and how those map to stream IDs.
  • IConnectionStream and IConnection are split in the proposed API specifically to later support an IMultiplexedConnection that creates multiple IConnectionStream.
    • To avoid an IMultiplexedConnection, we might choose to merge the two APIs and have IConnection force users to explicitly open the one bidirectional IConnectionStream for the connection.

A QUIC extension to this might look like:

interface IMultiplexedConnection : IAsyncDisposable, IDisposable
{
    EndPoint? RemoteEndPoint { get; }
    EndPoint? LocalEndPoint { get; }

    ValueTask<IConnectionStream> OpenStream(IConnectionProperties? options = null, CancellationToken cancellationToken = default);
    ValueTask<IConnectionStream> AcceptStream(IConnectionProperties? options = null, CancellationToken cancellationToken = default);
}

interface IMultiplexedConnectionFactory : IAsyncDisposable, IDisposable
{
    ValueTask<IMultiplexedConnection> ConnectAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);
    ValueTask<IMultiplexedConnectionListener> BindAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);
}

interface IMultiplexedConnectionListener : IAsyncDisposable, IDisposable
{
    IConnectionProperties ListenerProperties { get; }
    EndPoint? LocalEndPoint { get; }
    ValueTask<IMultiplexedConnection> AcceptAsync(IConnectionProperties? options = null, CancellationToken cancellationToken = default);
}

Alternately, the IConnection and IMultiplexedConnection APIs might be merged, reducing surface area significantly. The TCP version would simply throw if opening/accepting more than once. This API might look like:

interface IConnection
{
    EndPoint? LocalEndPoint { get; }
    EndPoint? RemoteEndPoint { get; }

    ValueTask<IConnectionStream> OpenStream(IConnectionProperties? options = null, CancellationToken cancellationToken = default);
    ValueTask<IConnectionStream> AcceptStream(IConnectionProperties? options = null, CancellationToken cancellationToken = default);
}

However, beyond API surface reduction there isn't clear practical reason to merge them. Given how QUIC is significantly different from TCP, it isn't clear that libraries would see correct reuse of filtering IConnectionFactory implementations.

Want: Pipelines support

Want: Pipelines support

It's there!

How many 3rd party libraries are in need of this and would use it if available?

MySqlConnector would probably use this. It currently has a homemade version of something similar: the base TCP socket or Unix domain socket connection can optionally have a SslStream-based connection layered on top of it (if the connection is secure), and either of those can have protocol compression layered on top of them. (I say "maybe" because the MySQL protocol has some quirks that make it not seamlessly composable.) There are also currently server load-balancing features that might be more cleanly expressed as another IConnectionFactory layer.

The connection factory could be exposed to advanced users for many of the same scenarios you've mentioned for HttpClient: bandwidth monitoring, protocol (SQL) inspection, in-memory (or alternate) transport, etc.

How many 3rd party libraries are in need of this and would use it if available?

Not 3rd party, but Orleans uses the Bedrock abstractions and we will use this when it becomes available. We can help validate the APIs early, if that may be useful to you.

Having both Stream and Pipe exposed is a little confusing to me - how will that work?

I say "maybe" because the MySQL protocol has some quirks that make it not seamlessly composable.

@bgrainger Can you give more detail? The goal is to have a very flexible model for composability, and it would be great to know if there's a flaw we can address.

Having both Stream and Pipe exposed is a little confusing to me - how will that work?

@ReubenBond Stream and Pipelines are both in broad use, and this is a way for us to enable wide adoption of the abstraction.

The intended behavior is that if only one is implemented, the other should wrap that. To avoid usage errors, once one propery is used calling the other will throw an exception.

We can help validate the APIs early, if that may be useful to you.

Awesome! We have some of our own validation planned already; I will ping you with details once it's in that stage.

This is complicated to explain briefly, but I'll try. Each packet in the MySQL protocol has a monotonically increasing "sequence number" in its header. Using compression in the protocol takes an arbitrary number of uncompressed packets, compresses them all together, then writes them as the payload of one or more compressed packets, which have their own sequence numbers. You might expect that this compression would be transparent to the underlying data, but it's not. When the uncompressed packets require more than one compressed packet (because the compressed data exceeds the 2^24 byte size limit), then the sequence numbers of the uncompressed packets get reset, starting from the sequence number of the compressed packet that contains them.

For example, if we have uncompressed packets 1-80, and packets 1-50 become the (compressed) payload of compressed packet 1, and packets 51-80 become the (compressed) payload of compressed packet 2, then packets 51-80 get renumbered 2-31 before compression. So there has to be coordination between the compression stream and the layers around it.

I started testing out the non-multiplexed connection abstractions in Kestrel's Socket transport, and here are a few things I noticed.

  • IConnectionProperties not having a setter is annoying.
  • IConnectionProperties.TryGet not having a generic version on the interface removes some opportunities for dead code elimination in custom implementations like Kestrel's TransportConnection.
  • No ConnectionId as a top level property on IConnection makes logging difficult. Right now, Kestrel just uses an IConnectionIdFeature interface in ConnectionProperties, but it's not great.
  • EDIT: IConnection not having an Abort() API means that Kestrel uses IConnectionLifetimeFeature.Abort() and stores the IConnectionLifetimeFeature in ConnectionProperties. When we dispose the Stream or complete the pipe, we still expect all the written data to be flushed. Abort() means kill the connection immediately even if everything isn't flushed. This can happen after the Stream is disposed or Pipe is completed. I feel this should be a top-level API.
    • Additionally, our experience in ASP.NET is that people dispose Streams when they're done using it, not necessarily when they want to close the connection.
  • No metadata on IConnectionListener is annoying. This makes it impossible for Kestrel to communicate what port it really bound to when asked to bind to port 0.
  • No CancellationToken for IConnectionListener.DisposeAsync/UnbindAsync could make it difficult to communicate to the listener when to switch from a graceful to an abortive shutdown, but right now Kestrel doesn't need this.

I haven't updated Kestrel's HttpsConnectionMiddleware to be a IConnectionListenerFactory decorator, but I could try that if people are interested.

I haven't tried out the multiplexed interfaces, but I think that having IMultiplexedConnection implement IConnectionStream will be problematic since neither the Pipe nor Stream properties will be usable.

Thanks for taking time to prove this out.

IConnectionProperties not having a setter is annoying.

I explicitly left this out to push a compositional model, but am interested in where it breaks down. Can you give some specific examples of how you'd use a setter?

IConnectionProperties.TryGet not having a generic version on the interface removes some opportunities for dead code elimination in custom implementations like Kestrel's TransportConnection.

FWIW we have loosely agreed to get rid of the type-based model all-together and instead have the property key just be object so there is no accidental shadowing. Available keys would be communicated via static properties similar to WPF dependency properties.

Can you give an example of where a generic version helps? We can revisit that decision if you've found it makes a significant difference.

  • IConnection not having an Abort() API means that Kestrel uses IConnectionLifetimeFeature.Abort() and stores the IConnectionLifetimeFeature in ConnectionProperties. How are we supposed to abort connections? Disposing the Stream? Our experience in ASP.NET is that people dispose Streams when they're done using it, not necessarily when they want to close the connection.

Beyond some Abort() method, there's also a consideration of how to communicate such an abort from the remote end to the local Stream or Pipe. How will kestrel react to e.g. a SocketException with one implementation vs a CustomProtocolResetException in another?

Right now, it's intentionally left undefined: our experience working with QUIC revealed a very TCP-centric mental model for aborting. Trying to make an abort API generic between just those two doesn't seem possible, let alone considering any other random protocol (does a UDS have compatible concepts of abort/reset/etc.?). Do you feel like a feature will work for now (at least, for this first pass in .NET 5), or do you think it's worth some brainstorming?

No CancellationToken for IConnectionListener.DisposeAsync/UnbindAsync could make it difficult to communicate to the listener when to switch from a graceful to an abortive shutdown, but right now Kestrel doesn't need this.

See above :)

No metadata on IConnectionListener is annoying. This makes it impossible for Kestrel to communicate what port it really bound to when asked to bind to port 0.

Agreed, it makes sense to add a property bag there.

I explicitly left this out to push a compositional model, but am interested in where it breaks down. Can you give some specific examples of how you'd use a setter?

Given that filters already need to wrap the IConnection to replace the Stream or Pipe, this isn't actually so bad by comparison. I think the pain came from adapting existing Kestrel code that expects a more mutable IConnection/ConnectionContext.

Kestrel's HttpsConnectionMiddleware adds ITlsApplicationProtocolFeature to ConnectionProperties. To do this HttpsConnectionMiddleware needs to wrap IConnection, IConnection.Stream/Pipe and IConnection.ConnectionProperties with custom implementations. This is more painful than just setting the Stream/Pipe on a mutable IConnection and adding a property to a mutable IConnectionProperties.

This is definitely not a huge issue though. I think it's mostly a matter of preference. With the wrapping approach, you don't need lines like context.Transport = originalTransport in a finally when unwinding. HttpsConnectionMiddleware doesn't even unset ITlsApplicationProtocolFeature, but with the wrapping approach this happens implicitly which is nice.

Can you give an example of where a generic version helps? We can revisit that decision if you've found it makes a significant difference.

It makes a bigger difference the more features/properties there are and the further down the else if (typeof(TFeature) == typeof(IFeatureInQuestion)) chain we check for the feature/property. @benaadams ran some microbenchmarks a while back aspnet/KestrelHttpServer#2290.

How will kestrel react to e.g. a SocketException with one implementation vs a CustomProtocolResetException in another?

Kestrel doesn't special-case exception types thrown from the transport except to log ConnectionResetExceptions (which are custom to Kestrel transports) at a lower level than most other IOExceptions.

Kestrel is probably somewhat unusual where it expects writing to the transport to never fail even if the connection has been closed by the client or aborted by the server app. The only way Kestrel can observe the connection failing is either by observing an exception when reading from the transport or a ConnectionClosed CancellationToken firing.

Kestrel will probably have to adapt any shared transport to have the unusual write-never-throws behavior, but I do think we should add a ConnectionClosed or StreamClosed CancellationToken to IConnectionStream instead of relying on something in ConnectionProperties.

Right now, it's intentionally left undefined: our experience working with QUIC revealed a very TCP-centric mental model for aborting. Trying to make an abort API generic between just those two doesn't seem possible, let alone considering any other random protocol (does a UDS have compatible concepts of abort/reset/etc.?). Do you feel like a feature will work for now (at least, for this first pass in .NET 5), or do you think it's worth some brainstorming?

Yeah. Abort() not taking an error code is problematic for QUIC. In Kestrel we added a new Abort() that takes an error code and default to 0x102 (H3_INTERNAL_ERROR) for the parameterless version. I agree it's not great. I'm OK with implementing Abort() via ConnectionProperties, but that doesn't feel great either.

It makes a bigger difference the more features/properties there are and the further down the else if (typeof(TFeature) == typeof(IFeatureInQuestion)) chain we check for the feature/property. @benaadams ran some microbenchmarks a while back aspnet/KestrelHttpServer#2290.

Ahh clever.

I also took a look at rewriting Kestrel’s HttpsConnectionMiddleware to use the proposed abstraction.

This went well aside from one key difference from the proposed SslConnectionListenerFactory which is that HttpsConnectionMiddleware wraps the IConnectionListener instead of IConnectionListenerFactory. The reason for this is that Kestrel needs to be able to configure connection middleware on a per-remote-endpoint basis. We could theoretically have a singleton SslConnectionListenerFactory keep track of mappings between each remote endpoint and its configuration, but Kestrel already easily does this, so I see no reason to make connection middleware do this given each middleware would likely have to come up with its own hand-rolled solution.

When rewriting the HttpsConnectionMiddleware, I wound up liking the wrapping the IConnection more than setting properties on ConnectionContext like we did before. It’s a little more verbose (about 33% more), but it really encourages writing low-allocation code, and it eliminates a class of bug where middleware doesn’t properly reset a property when exiting. If the middleware was simpler, I might find wrapping the IConnection a bit more onerous.

One thing that was really annoying when writing both the transport and middleware was writing the logic to throw when accessing the Stream after the IDuplexPipe and vice versa. This logic needs to be written in the transport, and then rewritten at each layer that wraps the Stream/IDuplexPipe. Maybe we should write some built in type that you can construct with either a Stream or an IDuplexPipe, exposes both types as properties that throws if the other property was accessed first.

One thing I really like is this design allows for the possibility of writing a connection throttling middleware since it allows intercepting the call to IConnectionListener.AcceptAsync(). If we use this, we won’t need to add yet another extensibility point to Kestrel for this functionality as suggested in dotnet/aspnetcore#13295.

API in issue updated with feedback from @stephentoub, @davidfowl, and @halter73

  • IConnectionProperties is no longer type-based, but rather uses a property system inspired by WPF dependency properties. The new system has improved discoverability, is strongly typed, and helps prevent unintentional shadowing that can happen when using types.
  • SocketsHttpHandler implementation is significantly changed; it now implements both "easy mode" callbacks and full IConnectionFactory support. For simplicity it no longer allows the user to override the TLS implementation, though this window is left cracked open to allow for a future API if desired.
  • Add a virtual CreateSocket to the sockets factories for users wanting to set socket options pre-connect.
  • Add ListenerProperties and LocalEndPoint to IConnectionListener.
  • Add IConnectionFactory.Filter extension methods for easily injecting a stream filter with a callback instead of implementing multiple interfaces.
  • Add ConnectionFactory property to SmtpClient.

CC @JamesNK

Results from SmtpClient proving

The API worked great here, with one caveat:

Only supporting asynchronous APIs presents a significant hurdle for existing APIs to adopt without costly sync-over-async. Not supporting synchronous Dispose is particularly challenging, as there is a lot of established code doing a non-async using on e.g. Stream.

We should consider implementing synchronous APIs to make this story better.

Why do we need sync APIs and which ones exactly do you mean?

Why do we need sync APIs and which ones exactly do you mean?

So here's the driving concern: how do we get these interfaces into existing APIs that have sync support?

For instance, SmtpClient.Send has a full synchronous path right now. It uses sync Socket.Connect there. The moment it moves over to this new abstraction, it no longer has that ability. Similarly, we might also have HttpClient's new sync APIs use a sync connect.

We also see IDisposable being implemented for a lot of APIs right now, and so only supporting IAsyncDisposable means all those APIs have no great way to dispose of an IConnection.

It leads us with a few options:

  • Add synchronous support to these interfaces. This complicates implementations, but makes just using it a lot more flexible.
  • Tell people that IConnectionFactory is just not compatible with sync code, and libraries using it as an extensibility point should throw if you use sync APIs combined with a factory. This kills the usefulness of the API: instead of replacing code, it will just add more.
  • Tell people to use sync-over-async.

I don't like any of these options, but we need to pick one. I'm leaning toward adding synchronous support being the least worst.

Then we never move forward because someone somewhere is pretending that async networking is actually sync.

If needed, have a different set of interfaces IBlockingConnectionFactory, IBlockingConnection, etc don't conflate the different and conflicting modes of operation in one set of interfaces.

Also then support can be determined at compile time; rather than either throwing or hand waving at runtime.

That is a better idea because then someone writing a transport doesn't have to provide the sync methods.

I think adding blocking networking APIs adds a lot of complexity for little benefit. As @Drawaes says, networking inherently async. I understand that the OS exposes blocking APIs, but I don't think that suddenly makes the blocking OK.

We should always be telling developers to do non-blocking I/O if they want to build a highly scalable apps. Blocking I/O still leads to threadpool starvation much faster than non-blocking I/O even without any sync-over-async. For developers who are porting low-traffic line-of-business apps that have always managed doing blocking I/O without threadpool starvation, I think sync-over-async is likely fine.

We could always go back and add IBlocking variants of the interfaces as @benaadams suggests if there's enough demand.

Lots of expert-focused opinions here. I think we can all agree async is what we'd prefer, but lets make sure we are looking at it from other perspectives as well. I don't view this as an expert-only API.

One of the goals of this is adoption into existing libraries. Lack of blocking APIs will hurt that goal.

We also have data (as outlined in HttpClient sync issue) that many users do not understand effective async, and that they still desire high performance sync APIs. Some are okay with horizontal rather than vertical scalability if it simplifies their code. There is value in enabling this scenario.

One of the goals of this is adoption into existing libraries. Lack of blocking APIs will hurt that goal.

Lets be specific though, lets talk about specific libraries and get some code samples. You might be right but lets get some concrete data about which libraries.

Can you outline the sync APIs you want to add?

Lets be specific though, lets talk about specific libraries and get some code samples.

SmtpClient and HttpClient.

Both need a sync Connect() mechanism, both keep a socket around that needs cleanup inside of their Dispose() methods.

We can implement IAsyncDisposable on both if we want, but this won't erase the large amount of existing user code that is doing a non-async using.

Can you outline the sync APIs you want to add?

Connect() and Dispose() for everything under IConnectionFactory. I see less use for blocking listener methods so I don't have strong opinions there, but consistency is worth considering.

HttpClient only just had the sync api added; and it isn't highly discoverable. Also you shouldn't be newing up and disposing HttpClient per request in any high usage scenario as you will be leaving lots of sockets in a TIME_WAIT state for 240 seconds. So it would only be low throughput scenerios when threadpool exhaustion due to Connect sync-over-async would be less important?

Not sure about SmtpClient does it have an async api?

IConnectionProperties is no longer type-based, but rather uses a property system inspired by WPF dependency properties. The new system has improved discoverability, is strongly typed, and helps prevent unintentional shadowing that can happen when using types.

I don't quite understand the benefits here. Improved discovery ability how? Strongly typed how? Can you show a before and after example of how it improved each of these?

HttpClient only just had the sync api added; and it isn't highly discoverable.

The new sync APIs are very discoverable first-class APIs on HttpClient.

And Dispose() already exists, which now needs to call IConnection.DisposeAsync().

Also you shouldn't be newing up and disposing HttpClient per request in any high usage scenario as you will be leaving lots of sockets in a TIME_WAIT state for 240 seconds. So it would only be low throughput scenerios when threadpool exhaustion due to Connect sync-over-async would be less important?

Yes, it's certainly sub-optimal but I think it's not the end of the world for typical HttpClient usage.

Keeping in mind that I'm not just worried purely about efficiency, but about adoptability too. Any library using this will need to know how to do sync-over-async against ValueTask, and due to lack of understanding around this we currently prefer APIs to only ever expose a ValueTask if the only thing we expect users to do with it is to directly await it.

Not sure about SmtpClient does it have an async api?

Both sync and async, with more or less the same usage pattern as HttpClient.

IConnectionProperties is no longer type-based, but rather uses a property system inspired by WPF dependency properties. The new system has improved discoverability, is strongly typed, and helps prevent unintentional shadowing that can happen when using types.

I don't quite understand the benefits here. Improved discovery ability how? Strongly typed how? Can you show a before and after example of how it improved each of these?

Before:

// zero discoverability -- no way to know if this key exists without reading docs.
if(properties.TryGet(out ISslConnectionInformation? sslInfo))
{
   // ...
}

// above is just a helper method that translates to this...
if(properties.TryGet(typeof(ISslConnectionInformation), out object? sslInfoObject) && sslInfoObject is ISslConnectionInformation sslInfo)
{
   // ...
}

After:

class SslConnectionFactory : IConnectionFactory
{
    // a new pattern factories should follow: property keys are used to lookup properties and can be discovered easily just by reading ref source. inspired by WPF dependency properties.
    public static ConnectionPropertyKey<ISslConnectionInformation> ConnectionInformationProperty { get; }
}

// no more object, and you can't accidentally do e.g. "out int".
if(properties.TryGet(SslConnectionFactory.ConnectionInformationProperty, out ISslConnectionInformation? sslInfo))
{
   // ...
}

The new sync APIs are very discoverable first-class APIs on HttpClient.

Not they aren't. If they were we'd had more overloads (matching the async ones). We intentionally made it "more advanced" because it's not the ideal or recommended usage.

@scalablecory That example isn't convincing to me. How did you know to look up by a static instance property on a random type?

@scalablecory That example isn't convincing to me. How did you know to look up by a static instance property on a random type?

I think the common use case will be people knowing exactly which factories they're using, and this gives them a path to finding which properties they expose/expect.

For the extensibility scenario, e.g. a custom transport plugin, this pattern could be extended to have HttpClient expose its own properties to indicate what transports might choose to implement.

SmtpClient is obsolete

SmtpClient is not obsolete. It's a bug in the docs.
dotnet/dotnet-api-docs#2986

Aggregating current outstanding discussions from here and in-person meetings. If I'm misrepresenting anyone's opinions here please edit with corrections.

  • @halter73 believes the changes made addressed his concerns.
  • The new property bag is a controversial style and @scalablecory would like some more feedback on it.
    • Type-based vs object-based keying:
      • @stephentoub called out that in the original design, with properties keyed off of type, that you can not expose two properties with the same type and that factories might accidentally shadow each-other.
      • @davidfowl, @halter73 would prefer the Type-based approach, having seen this approach be usable enough in practice within Kestrel. Suggested workaround for @stephentoub's concern is to implement a factory-specific interface i.e. instead of two EndPoint properties, have ISocketRemoteEndPoint, ISocketLocalEndPoint, or ISocketEndPoints.
      • @scalablecory likes the discoverability benefits of having explicit key objects, but acknowledges that this may be less useful in ASP/NET / DI-heavy scenarios. @davidfowl does not believe this will be useful for him.
    • Generic virtuals have more overhead than non-generic virtuals. It would be good to understand how often the property bag will be queried in practice. For reference, current ASP.NET feature collection type uses a generic version too.
      • It's also weird for users to implement: they need to cast their type to object and then to generic. It's free from perf perspective, but weird to type.
  • "Factory" naming is ambiguous between e.g. static factory method and a factory class. Is there a better name we can use to disambiguate?
  • Interface-heavy and DI-ish dynamic property design is not common to BCL. Do we have data suggesting users would be successful with this, to help guide if this is the direction BCL should take?
  • Consider getting rid of ConnectionPropertiesCollection. It provides some usefulness in having a default "safe" implementation for users who want less ceremony, but perhaps it doesn't provide enough value.
  • Consider merging separate connect/bind Sockets and Ssl factories into single factories that implement two interfaces.
  • Do we need a default Socket implementation?
    • Libraries needing to e.g. set their own socket options will need to derive and make their own types anyway, perhaps it is okay to force them to implement the entire thing themselves.
    • Getting rid of it may significantly raise barrier of entry, harming adoptability scenario.
  • If we go the "pure" route of removing the Local/Remote endpoint properties, we can get rid of IConnection type and might have better code sharing between QUIC and TCP implementations.
    • If we go further and have force a call-once OpenStream on IConnection for TCP, we could further merge the TCP and QUIC APIs and enable more reuse.
    • If we go even further and merge the two factory interfaces, we would enable better code sharing for middleware.
    • @scalablecory, based on HTTP/3 experience, is skeptical about how much code could truly be shared across TCP and QUIC layers.
  • Should we support synchronous APIs? This is an adoptability concern: we would like new libraries to be async-only, but recognize that migrating old sync APIs to this new abstraction will be difficult. There is no "good" option, only tradeoffs:
    • @davidfowl @halter73 prefers to recommend sync-over-async; the overhead is probably not bad (especially when only for connection establishment, not actual data stream).
      • con: People have trouble doing sync-over-async properly and it's even harder with ValueTask, so usability is hurt here.
    • @stephentoub prefers for libraries to throw when users provide a custom factory, but try to use sync APIs. Recommends implementing IDisposable but no other sync APIs.
      • con: This forces existing libraries to dual-path in a non-trivial way: keep their old sync code around, and use these abstractions for their async path.
    • @scalablecory prefers (not strongly) sync methods. At the bare minimum, would like IDisposable as too much existing code depends on synchronous using.
      • con: This forces IFactory authors to implement all their methods twice.
      • con: ASP.NET is fully async and strongly discourages users writing sync code, so they would have to think about e.g. NotImplementedException or some other mechanism.
    • This would encourage writing async-only networking code for new libraries.
    • Anyone who tries to do sync-over-async would not have a great time (expert knowledge needed).
    • This will harm adoptability by forcing existing libraries to dual-path their connection establishment logic: one using this for async, one using old non-abstracted code.

Just curious, shouldn't the namespace be System.IO.Connections as it could interact with different types of transports like serial ports, buses (I2C, SPI, GPIO, etc.), Bluetooth, etc.?

Just curious, shouldn't the namespace be System.IO.Connections as it could interact with different types of transports like serial ports, buses (I2C, SPI, GPIO, etc.), Bluetooth, etc.?

You make a good point. We shouldn't consider namespace final; some things will change here anyway,

API Updated @JamesNK @halter73

Changes:

  • Reverted property bag back to being Type-based. Reason being: I want to make sure we do not bifurcate patterns used in ASP vs BCL.
    • Will be seeking advice from API review team on discoverability issue.
    • Alternate is described in #1793 (comment), but might be made to use Object keys rather than a custom type. This would result in Kestrel doing TryGet(typeof(IFoo), ...) and BCL doing TryGet(Factory.FooProperty, ...), though, so usage patterns would be different.
  • Added IDisposable to all the types taking IAsyncDisposable.
  • Merged SocketsConnectionFactory and SocketsConnectionListenerFactory.
  • Merged SslConnectionFactory and SslConnectionListenerFactory.
  • Added SocketsConnectionFactory virtuals for creating Socket, Stream, and IDuplexPipe.
  • Renamed SocketsHttpHandler.SetFilter to SetConnectionFilter, for consistency with SetConnectionFactory and to avoid confusion between per-connection and per-request filtering.

More notes:

  • We initially removed the HttpsMiddleware, preferring users to not worry about complexity of inserting TLS implementation. We'd like to revisit this, possibly by enabling this feature directly as part of SslConnectionFactory. More investigation needed here. Will not block review on it though: will tackle this in a subsequent API proposal.

@scalablecory are there any preview packages we can use to start playing around with? Of course, with the understanding this is early and changes are expected. Any help is appreciated.

@scalablecory are there any preview packages we can use to start playing around with? Of course, with the understanding this is early and changes are expected. Any help is appreciated.

We're aiming to get this into a preview soon, once API review has been done.

@scalablecory Had troubles finding the review today. Could you add the link once posted on YouTube? Thx

The API review was rescheduled for Thursday.

In .NET, we try to avoid: a) interfaces, b) factories. Could this work as follows?

// server
using var socket = new Socket(SocketType.Stream, ProtocolType.Tcp);
await socket.BindAsync(new IPEndPoint(IPAddress.IPv6Loopback, 0), cancellationToken);
using SocketConnection connection = socket.AcceptConnectionAsync(cancellationToken);
using Stream stream = connection.Stream;

// client
using var socket = new Socket(SocketType.Stream, ProtocolType.Tcp);
using SocketConnection = await socket.OpenConnectionAsync(new DnsEndPoint("contoso.com", 80), cancellationToken);
using Stream stream = connection.Stream;

In .NET, we try to avoid: a) interfaces

Have default interfaces methods changed the guidelines?
Interfaces are now almost the same as abstract classes, except that classes can only be derived from once.

API review notes:

  • There was an unresolved discussion that most, if not all, of these interfaces should be abstract classes.
  • ConnectionPropertyCollection shouldn't use the name Collection since it's not enumerable
  • (I)ConnectionStream should not use the name Stream, since it does not derive from Stream.
  • There was an unresolved discussion around splitting the connection factory into two types, one for Connect and one for Bind.
    • The main question was about what that would look like for implementations, and if it becomes limiting for overload resolution--including extension method resolution.
    • If they're on the same type, is it worth a CanBind/CanAccept?
  • There was a long, unresolved, debate of whether or not the connection factory should be disposable.
    • What should the ownership model of factory composition be? If the factory type abstraction isn't disposable but a specific factory is, who owns it now?
  • Consider an Abort() method on IConnectionStream.
    • Presumably that requires implementers to have a way to abort their stream or pipe?
    • Does that make Dispose() graceful, or do we need some kind of CloseGracefully() to disambiguate that as well?
  • Combine IConnectionStream and IConnection, the endpoint properties are already nullable so the separation isn't providing a lot of value.
  • There was a long, unresolved, debate on using Type or object as the key on the property lookup.

One potential alternative for the property lookup (in no way agreed upon, but took elements from the discussion):

public abstract class Connection
{
    public bool TryGetProperty<T>(out T value) where T : IConnectionPropertySet
    {
        object valueObject;

        if (!TryGetProperty(typeof(T), out valueObject))
        {
            return _parent != null && _parent.TryGetProperty(typeof(T), out valueObject);
        }

        value = (T)valueObject;
        return true;
    }

    public T GetProperty<T>() where T : IConnectionPropertySet
    {
        if (!TryGetProperty<T>(out T value))
        {
            throw Something;
        }

        return value;
    }

    protected virtual bool TryGetProperty(Type key, out T value);
}

unresolved discussion that most, if not all, of these interfaces should be abstract classes.

If you have both server and client in an abstract class; then you don't have compile time help when only one is implemented. If they are in separate classes then you can't implement both. Also if they are classes then you can't inherit from SafeHandle; so would need a secondary indirection for everything if you did use it.

If there is an interface for server and client; then you get explicit compile time help if you only implement one; you can implement SafeHandle directly etc.

<T> form of GetProperty is better than an Type, object as you'd (should) want to cast straight away; so the loose typing isn't great.

If they are in separate classes then you can't implement both.

Depending on what extension methods and overloads and the like existed, that'd actually be a goal. For interface-based approaches, if there were ambiguous-target methods, it would probably be good to write an analyzer that says to not implement both of them... just to avoid the problems later.

@scalablecory, sorry for question out of the blue. Would it be possible to monitor connectivity state of underlying connections? Assuming that client keeps multiple of them, I would like to know their state, possibly using reactive approach based on observer pattern.

@wicharypawel a connection could implement some sort of IConnectionMonitor property, but the in-box ones will currently not -- the general guidance will be to create Nuget packages that can insert themselves into the layering to expose such features.

@scalablecory thank you for your answer, as long as this would be possible to implement, it's fine to me.

I'm still a little confused on how the TryGetProperty will work. What if you have multiple properties of the same Type, how would it know when to get a specific property?

This question is similar to @stephentoub concerns in 1st review.

For example, if I have 2 properties of type int (i.e. int baudRate and int dataBits). How would you get each property individually since they are both int?

Current gist:

public enum ConnectionCloseMethod
{
    GracefulShutdown,
    Abort,
    Immediate
}

public abstract class ConnectionBase : IAsyncDisposable, IDisposable
{
    public virtual IConnectionProperties ConnectionProperties { get; }
    public virtual EndPoint? LocalEndPoint { get; }
    public virtual EndPoint? RemoteEndPoint { get; }

    public abstract ValueTask CloseAsync(ConnectionCloseMethod method = default, CancellationToken cancellationToken = default);
}

public abstract class Connection : ConnectionBase
{
    public Stream Stream { get; }
    public IDuplexPipe Pipe { get; }

    protected virtual Stream CreateStream();
    protected virtual IDuplexPipe CreatePipe();

    public static Connection FromStream(Stream stream, IConnectionProperties? properties = null, EndPoint? localEndPoint = null, EndPoint? remoteEndPoint = null);
    public static Connection FromPipe(IDuplexPipe pipe, IConnectionProperties? properties = null, EndPoint? localEndPoint = null, EndPoint? remoteEndPoint = null);
}

public abstract class ConnectionFactory : IAsyncDisposable, IDisposable
{
	public abstract ValueTask<Connection> ConnectAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);
	
	public void Dispose();
	public ValueTask DisposeAsync();
	
	protected abstract void Dispose(bool disposing);
	protected abstract ValueTask DisposeAsyncCore();
}

public abstract class ConnectionListenerFactory : IAsyncDisposable, IDisposable
{
	public abstract ValueTask<ConnectionListener> BindAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);
	
	public void Dispose();
	public ValueTask DisposeAsync();
	
	protected abstract void Dispose(bool disposing);
	protected abstract ValueTask DisposeAsyncCore();
}

public abstract class ConnectionListener : IAsyncDisposable, IDisposable
{
	public IConnectionProperties ListenerProperties { get; }
	public EndPoint? LocalEndPoint { get; }
    
	public void Dispose();
	public ValueTask DisposeAsync();

	protected abstract void Dispose(bool disposing);
	protected abstract ValueTask DisposeAsyncCore();
    
	public abstract ValueTask<Connection> AcceptAsync(IConnectionProperties? options = null, CancellationToken cancellationToken = default);
}

public class SocketsHttpConnectionFactory : ConnectionFactory
{
        // For users of the above two APIs, they can call this if they just want to wrap the defaults.
        public virtual Socket CreateSocket(HttpRequestMessage message, DnsEndPoint endPoint, IConnectionProperties options);
        public virtual ValueTask<IConnection> EstablishConnection(HttpRequestMessage message, DnsEndPoint endPoint, IConnectionProperties options, CancellationToken cancellationToken);
}

EstablishConnection returns an IConnection. Should it be ConnectionBase?

Are connections created and connected at the same time? What happens if a connection is disconnected? e.g. the server or proxy closes a connection. Do you reconnect using the existing connection, or should a new connection be created?

If connections support various states, should ConnectionBase have some kind of state property? i.e. idle, connecting, ready, closed. That would allow code to decide whether a connection can be used.

public enum ConnectionState
{
    Idle,
    Connecting,
    Ready,
    Closing,
    TransientFailure
}

public abstract class ConnectionBase : IAsyncDisposable, IDisposable
{
    public abstract ConnectionState State { get; }
}

These states are copied from how gRPC abstracts connection state. See https://github.com/grpc/grpc/blob/master/doc/connectivity-semantics-and-api.md

EstablishConnection returns an IConnection. Should it be ConnectionBase?

It should be Connection; thanks.

Should ConnectionBase have some kind of status? i.e. idle, connecting, ready, closed. That would allow code to decide whether a connection can be used.

It looks like this state could be tracked/deduced based on the current API. To make it pay-for-play (not all users would want overhead needed to track these things) I would suggest implementing some sort of StateMonitoringConnectionFactory that performs this work and exposes the state as a connection property.

I think I edited this paragraph in after you started replying:

Are connections created and connected at the same time? What happens if a connection is disconnected? e.g. the server or proxy closes a connection. Do you reconnect using the existing connection, or should a new connection be created?

For example, there is a ConnectionBase.CloseAsync but no ConnectionBase.ConnectAsync or state.

As a user of this, I'm trying to figure out whether out whether I should be interacting with the socket to connect, disconnect, reconnect, determine state, etc. Or should this be functionality exposed on the connection abstraction?

I think I edited this paragraph in after you started replying:

Are connections created and connected at the same time? What happens if a connection is disconnected? e.g. the server or proxy closes a connection. Do you reconnect using the existing connection, or should a new connection be created?

For example, there is a ConnectionBase.CloseAsync but no ConnectionBase.ConnectAsync or state.

As a user of this, I'm trying to figure out whether out whether I should be interacting with the socket to connect, disconnect, reconnect, determine state, etc. Or should this be functionality exposed on the connection abstraction?

ConnectionFactory.ConnectAsync() will return an already connected Connection. Once closed, the Connection can't be reused.

Were you to track the state yourself, it would look like:

// disconnected
Task ct = ConnectAsync()
// connecting
await ct;
// connected
Task dt = DisposeAsync();
// disconnecting
await dt;
// disconnected

with "idle" needing to wrap the Stream.Read etc. with some opinionated timers.

Current gist:

public abstract class ConnectionBase : IAsyncDisposable, IDisposable
{
    protected ConnectionBase();
    public abstract IConnectionProperties ConnectionProperties { get; }
    public abstract EndPoint? LocalEndPoint { get; }
    public abstract EndPoint? RemoteEndPoint { get; }
    public ValueTask CloseAsync(ConnectionCloseMethod method = ConnectionCloseMethod.GracefulShutdown, CancellationToken cancellationToken = default);
    protected abstract ValueTask CloseAsyncCore(System.Net.Connections.ConnectionCloseMethod method, CancellationToken cancellationToken);
}

public enum ConnectionCloseMethod
{
    GracefulShutdown,
    Abort,
    Immediate,
}

// needs review: FromPipe with disposable.
public abstract class Connection : ConnectionBase
{
    protected Connection();
    public IDuplexPipe Pipe { get; }
    public Stream Stream { get; }
    protected virtual IDuplexPipe CreatePipe();
    protected virtual Stream CreateStream();
    public static Connection FromPipe(IDuplexPipe pipe, bool leaveOpen = false, IConnectionProperties? properties = null, EndPoint? localEndPoint = null, EndPoint? remoteEndPoint = null);
    public static Connection FromStream(Stream stream, bool leaveOpen = false, IConnectionProperties? properties = null, EndPoint? localEndPoint = null, EndPoint? remoteEndPoint = null);
}

public static class ConnectionExtensions
{
    public static ConnectionFactory Filter(this ConnectionFactory factory, Func<Connection, IConnectionProperties?, CancellationToken, ValueTask<Connection>> filter);
    public static bool TryGet<T>(this IConnectionProperties properties, [MaybeNullWhenAttribute(false)] out T property);
}

public abstract class ConnectionFactory : IAsyncDisposable, IDisposable
{
    protected ConnectionFactory();
    public abstract ValueTask<Connection> ConnectAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);
    protected virtual void Dispose(bool disposing);
    protected virtual ValueTask DisposeAsyncCore();
}

public abstract class ConnectionListener : IAsyncDisposable, IDisposable
{
    protected ConnectionListener();
    public abstract IConnectionProperties ListenerProperties { get; }
    public abstract EndPoint? LocalEndPoint { get; }
    public abstract ValueTask<Connection> AcceptAsync(IConnectionProperties? options = null, CancellationToken cancellationToken = default);
    protected virtual void Dispose(bool disposing);
    protected virtual ValueTask DisposeAsyncCore();
}

public abstract class ConnectionListenerFactory : IAsyncDisposable, IDisposable
{
    protected ConnectionListenerFactory();
    public abstract ValueTask<ConnectionListener> BindAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);
    protected virtual void Dispose(bool disposing);
    protected virtual ValueTask DisposeAsyncCore();
}

public interface IConnectionProperties
{
    bool TryGet(Type propertyKey, [NotNullWhenAttribute(true)] out object? property);
}

// needs review. adding to existing class.
public class SocketsHttpHandler
{
    public ConnectionFactory? ConnectionFactory { get; set; }
    public Func<HttpRequestMessage, Connection, CancellationToken, ValueTask<Connection>>? PlaintextFilter { get; set; }
}

// needs review
public class SocketsHttpConnectionFactory : ConnectionFactory
{
    public SocketsHttpConnectionFactory();
    public sealed override ValueTask<Connection> ConnectAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);
    protected virtual Socket CreateSocket(HttpRequestMessage message, EndPoint? endPoint, IConnectionProperties options);
    protected virtual ValueTask<Connection> EstablishConnectionAsync(HttpRequestMessage message, EndPoint? endPoint, IConnectionProperties options, CancellationToken cancellationToken);
}

// needs review.
class SocketsConnectionFactory : ConnectionFactory
{
    // dual-mode IPv6 socket. See Socket(SocketType socketType, ProtocolType protocolType)
    public SocketsConnectionFactory(SocketType socketType, ProtocolType protocolType);

    // See Socket(AddressFamily addressFamily, SocketType socketType, ProtocolType protocolType)
    public SocketsConnectionFactory(AddressFamily addressFamily, SocketType socketType, ProtocolType protocolType);

    public override ValueTask<Connection> ConnectAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);

    // These exist to provide an easy way to shim the default behavior.
    // A more idiomatic (but more API-heavy) way to do this would be to pass some sort of ISocketConfiguration that has all the pre-connect socket options one could want.
    protected virtual Socket CreateSocket(AddressFamily addressFamily, SocketType socketType, ProtocolType protocolType, EndPoint? endPoint, IConnectionProperties? options);
    protected virtual Stream CreateStream(Socket socket, IConnectionProperties? options);
    protected virtual IDuplexPipe CreatePipe(Socket socket, IConnectionProperties? options);
}

// needs review.
class SocketsListenerFactory : ConnectionListenerFactory
{
    // dual-mode IPv6 socket. See Socket(SocketType socketType, ProtocolType protocolType)
    public SocketsListenerFactory(SocketType socketType, ProtocolType protocolType);

    // See Socket(AddressFamily addressFamily, SocketType socketType, ProtocolType protocolType)
    public SocketsListenerFactory(AddressFamily addressFamily, SocketType socketType, ProtocolType protocolType);

    public override ValueTask<ConnectionListener> BindAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);

    // These exist to provide an easy way for users to override default behavior.
    // A more idiomatic (but more API-heavy) way to do this would be to pass some sort of ISocketConfiguration that has all the pre-connect socket options one could want.
    protected virtual Socket CreateSocket(AddressFamily addressFamily, SocketType socketType, ProtocolType protocolType, EndPoint? endPoint, IConnectionProperties? options);
}

Video

  • Approved. We're done with it!
public abstract class ConnectionBase : IAsyncDisposable, IDisposable
{
    protected ConnectionBase();
    public abstract IConnectionProperties ConnectionProperties { get; }
    public abstract EndPoint? LocalEndPoint { get; }
    public abstract EndPoint? RemoteEndPoint { get; }
    public ValueTask CloseAsync(ConnectionCloseMethod method = ConnectionCloseMethod.GracefulShutdown, CancellationToken cancellationToken = default);
    protected abstract ValueTask CloseAsyncCore(System.Net.Connections.ConnectionCloseMethod method, CancellationToken cancellationToken);
}

public enum ConnectionCloseMethod
{
    GracefulShutdown,
    Abort,
    Immediate,
}

// needs review: FromPipe with disposable.
public abstract class Connection : ConnectionBase
{
    protected Connection();
    public IDuplexPipe Pipe { get; }
    public Stream Stream { get; }
    protected virtual IDuplexPipe CreatePipe();
    protected virtual Stream CreateStream();
    public static Connection FromPipe(IDuplexPipe pipe, bool leaveOpen = false, IConnectionProperties? properties = null, EndPoint? localEndPoint = null, EndPoint? remoteEndPoint = null);
    public static Connection FromStream(Stream stream, bool leaveOpen = false, IConnectionProperties? properties = null, EndPoint? localEndPoint = null, EndPoint? remoteEndPoint = null);
}

public static class ConnectionExtensions
{
    public static ConnectionFactory Filter(this ConnectionFactory factory, Func<Connection, IConnectionProperties?, CancellationToken, ValueTask<Connection>> filter);
    public static bool TryGet<T>(this IConnectionProperties properties, [MaybeNullWhenAttribute(false)] out T property);
}

public abstract class ConnectionFactory : IAsyncDisposable, IDisposable
{
    protected ConnectionFactory();
    public abstract ValueTask<Connection> ConnectAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);
    protected virtual void Dispose(bool disposing);
    protected virtual ValueTask DisposeAsyncCore();
}

public abstract class ConnectionListener : IAsyncDisposable, IDisposable
{
    protected ConnectionListener();
    public abstract IConnectionProperties ListenerProperties { get; }
    public abstract EndPoint? LocalEndPoint { get; }
    public abstract ValueTask<Connection> AcceptAsync(IConnectionProperties? options = null, CancellationToken cancellationToken = default);
    protected virtual void Dispose(bool disposing);
    protected virtual ValueTask DisposeAsyncCore();
}

public abstract class ConnectionListenerFactory : IAsyncDisposable, IDisposable
{
    protected ConnectionListenerFactory();
    public abstract ValueTask<ConnectionListener> BindAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);
    protected virtual void Dispose(bool disposing);
    protected virtual ValueTask DisposeAsyncCore();
}

public interface IConnectionProperties
{
    bool TryGet(Type propertyKey, [NotNullWhenAttribute(true)] out object? property);
}

// needs review. adding to existing class.
public class SocketsHttpHandler
{
    public ConnectionFactory? ConnectionFactory { get; set; }
    public Func<HttpRequestMessage, Connection, CancellationToken, ValueTask<Connection>>? PlaintextFilter { get; set; }
}

// needs review
public class SocketsHttpConnectionFactory : ConnectionFactory
{
    public SocketsHttpConnectionFactory();
    public sealed override ValueTask<Connection> ConnectAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);
    protected virtual Socket CreateSocket(HttpRequestMessage message, EndPoint? endPoint, IConnectionProperties options);
    protected virtual ValueTask<Connection> EstablishConnectionAsync(HttpRequestMessage message, EndPoint? endPoint, IConnectionProperties options, CancellationToken cancellationToken);
}

// needs review.
class SocketsConnectionFactory : ConnectionFactory
{
    // dual-mode IPv6 socket. See Socket(SocketType socketType, ProtocolType protocolType)
    public SocketsConnectionFactory(SocketType socketType, ProtocolType protocolType);

    // See Socket(AddressFamily addressFamily, SocketType socketType, ProtocolType protocolType)
    public SocketsConnectionFactory(AddressFamily addressFamily, SocketType socketType, ProtocolType protocolType);

    public override ValueTask<Connection> ConnectAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);

    // These exist to provide an easy way to shim the default behavior.
    // A more idiomatic (but more API-heavy) way to do this would be to pass some sort of ISocketConfiguration that has all the pre-connect socket options one could want.
    protected virtual Socket CreateSocket(AddressFamily addressFamily, SocketType socketType, ProtocolType protocolType, EndPoint? endPoint, IConnectionProperties? options);
    protected virtual Stream CreateStream(Socket socket, IConnectionProperties? options);
    protected virtual IDuplexPipe CreatePipe(Socket socket, IConnectionProperties? options);
}

// needs review.
class SocketsListenerFactory : ConnectionListenerFactory
{
    // dual-mode IPv6 socket. See Socket(SocketType socketType, ProtocolType protocolType)
    public SocketsListenerFactory(SocketType socketType, ProtocolType protocolType);

    // See Socket(AddressFamily addressFamily, SocketType socketType, ProtocolType protocolType)
    public SocketsListenerFactory(AddressFamily addressFamily, SocketType socketType, ProtocolType protocolType);

    public override ValueTask<ConnectionListener> BindAsync(EndPoint? endPoint, IConnectionProperties? options = null, CancellationToken cancellationToken = default);

    // These exist to provide an easy way for users to override default behavior.
    // A more idiomatic (but more API-heavy) way to do this would be to pass some sort of ISocketConfiguration that has all the pre-connect socket options one could want.
    protected virtual Socket CreateSocket(AddressFamily addressFamily, SocketType socketType, ProtocolType protocolType, EndPoint? endPoint, IConnectionProperties? options);
}

So I have a whole class to do my Setup when I just needed a method, ConnectionFactory does a lot of stuff, would love to see some implementations of it soon, does it have return semantics or I guess that all behind the scenes.

xqrzd commented

Looks pretty interesting. I have a bunch of obscure use cases that I'm curious what would look like using this (the context is a client for Apache Kudu).

  1. Apache Kudu does TLS and GSSAPI authentication wrapped in protbuf messages. The current setup looks like,

    Socket => NetworkStream => StreamWrapper => SslStream

    then finally SslStream is turned into a pipe with Pipelines.Sockets.Unofficial.

    StreamWrapper is needed because I can't replace SslStream's inner stream. For the TLS negotiate phase, I need to 'man in the
    middle' SslStream to wrap/unwrap the results into protobuf messages. During this time StreamWrapper contains
    KuduTlsAuthenticationStream. Once negotiation is complete StreamWrapper's stream is replaced with NetworkStream.

    GSSAPI is similar, except I don't need the StreamWrapper hack, as NegotiateStream is just used for authentication, while TLS is
    needed for the lifetime of the connection. Is this any simpler with these new connection abstractions?

  2. Is this API missing PipeOptions?
    public static Connection FromStream(Stream stream, bool leaveOpen = false, IConnectionProperties? properties = null, EndPoint? localEndPoint = null, EndPoint? remoteEndPoint = null);

  3. For the same API as 2, would it be possible to create individual PipeWriter/PipeReader (instead of IDuplexPipe) from a stream? There's a very obscure bug (I think in the Kudu server) where if 2 RPCs are sent in the same call to Socket.WriteAsync(), I only get a response for the first one sent. I can work around this by using PipeWriter.Create() for writing (which has no batching, preventing the bug), and StreamConnection.GetReader() for reading, which yields better throughput and allows data to back up in the pipe.

  1. Is [Connection.FromStream] missing PipeOptions?

My assumption is that the Connection created by FromStream is going to lazily call PipeReader.Create(Stream, StreamPipeReaderOptions) and PipeWriter.Create(Stream, StreamPipeWriterOptions) when Connection.Pipe.Reader and Connection.Pipe.Writer are first accessed (if ever) respectively.

It makes sense for StreamPipeReaderOptions and StreamPipeWriterOptions to be configurable, but the method signature is already quite long. Maybe we can pull StreamPipeReaderOptions and StreamPipeWriterOptions out of the IConnectionProperties parameter and use those if they exist.

I can work around this by using PipeWriter.Create() for writing (which has no batching, preventing the bug),

Good news! I think that's what you're going to get.

and StreamConnection.GetReader() for reading, which yields better throughput and allows data to back up in the pipe.

Now that's too bad. I don't think you're going to get a DefaultPipeReader using Connection.FromStream(). For this kind of advanced use case, you'll probably need to implement Connection yourself.

  1. Is [Connection.FromStream] missing PipeOptions?

My assumption is that the Connection created by FromStream is going to lazily call PipeReader.Create(Stream, StreamPipeReaderOptions) and PipeWriter.Create(Stream, StreamPipeWriterOptions) when Connection.Pipe.Reader and Connection.Pipe.Writer are first accessed (if ever) respectively.

It makes sense for StreamPipeReaderOptions and StreamPipeWriterOptions to be configurable, but the method signature is already quite long. Maybe we can pull StreamPipeReaderOptions and StreamPipeWriterOptions out of the IConnectionProperties parameter and use those if they exist.

I can work around this by using PipeWriter.Create() for writing (which has no batching, preventing the bug),

Good news! I think that's what you're going to get.

and StreamConnection.GetReader() for reading, which yields better throughput and allows data to back up in the pipe.

Now that's too bad. I don't think you're going to get a DefaultPipeReader using Connection.FromStream(). For this kind of advanced use case, you'll probably need to implement Connection yourself.

Hope not otherwise whats the point? What does it then Abstract? Connect and Disconnect? maybe send and receive? I can have an interface ISocketRefererece{ IEnumerable<Socket>GetSockets();} and call it day. (which I did 10 years ago)

It sounds like you want to be able to change pipe implementation which sounds reasonable. Something we’ll need to decide

@scalablecory it occurs to me that we need a way to control pipe creation on the SocketsListenerFactory as well. Kesterl will want to use a different pipe implementation than the default (assuming the default will be network stream based).

@scalablecory it occurs to me that we need a way to control pipe creation on the SocketsListenerFactory as well. Kesterl will want to use a different pipe implementation than the default (assuming the default will be network stream based).

The default is not network stream based, but does lack some of the Kestrel-specific features (scheduler, throwing certain exceptions).

Right, that's a fine default but we'd like to override it. I think that should be possible or we'll need to write a custom transport which wouldn't be as ideal.

Closing this issue (the PR has been merged). Remaining work is tracked in #40044.

I have simple task - detect remote IP address to which http client is connected. I don't want to DNS resolve, but need exactly remote IP.
I didn't understand how to detect remote IP in net core 3.1.
Also I have no time for reading all of topics regarding breaking changes and new API.

I have found solution for .net - https://stackoverflow.com/questions/6655713/how-to-get-ip-address-of-the-server-that-httpwebrequest-connected-to but it doesn't work as it is for dot net, but not core.

Answer please, my simple question.
I can use WebRequest or HttpClient or anything you suggest.

Reopening this since we reverted it in 5.0

Will this include some form of Socket frame decoding/encoding support out of the box like netty in Java? One of my gripes with the .NET Socket is that you have to write so much boiler plate to get communications setup.

How many ProtocolType are we supposed to support? I would prefer: TCP, UDP, HTTP, WebSocket, gRPC, Protobuff, etc.

In application point of view, they are all communication level.

Understanding possible .NET 6 features/roadmap has not been released yet, anyone know when we might start seeing focus back on this API?

I'm going to close this. If we decide to revisit it later, it can be re-opened, but at present it's not actionable. Thanks.