Add AcceptSocketAsync to SocketTransportOptions

[kevin-montrose]

Add AcceptSocketAsync to SocketTransportOptions

Introduces a new option on SocketTransportOptions that allows customizing what happens between wanting to accept a new connection, and actually wrapping it up in a SocketConnection.

The motivating use case for this is to conditionally shove certain sockets to different processes based on peeking at the first packet or two.

Addresses #34344

[Tratcher]

Can you show an example of how this addresses your original issue? If you call DuplicateAndClose on the accepted socket then what do you return? Or do you not return and just loop back to another Accept internally?

[kevin-montrose]

@Tratcher yes, in the existing process if we marshal the socket to another process we DuplicateAndClose it and loop back to accept another.

More or less this:

while (true)
{
  cancellationToken.ThrowIfCancellationRequested();

  var newConnection = await listenSocket.AcceptAsync();

  if (!await TryRerouteSocketAsync(newConnection))
  {
    // we decided to handle the request in this process
    return newConnection;
  }
}

In the new process(es) we'd use some IPC mechanism to read marshalled Sockets and return them AcceptSocketAsync. In my exploration, I've accepted one connection off the normal listen socket and used that as the IPC mechanism.

A little more complicated, but at a high level something like:

  this.EnsureReceiving(listenSocket);  // spins up some connection to receive sockets, and newly received Sockets are written to receiveChannel.Writer

  var socket = await receiveChannel.Reader.ReadAsync(cancellationToken);

  return socket;

[shirhatti]

@JunTaoLuo Did you ever prototype your RateLimiting stuff with the socket accept loop? Will this compose well with your prototype? Do you have any concerns?

[JunTaoLuo]

I didn't have a chance to prototype with the socket accept loop specifically, but looking at this change, it would compose with the rate limiter APIs. Specifically, the AcceptSocketAsync method can be made to resolve a limiter to use when making the decision to when to accept the socket.

[halter73]

This seems related to #13295. I think if we did something like this, we'd probably want something like the UseListenerFilter proposed by @davidfowl here.

@kevin-montrose As a tactical way to achieve something like this, could you register a custom IConnectionListenerFactory that wraps SocketTransportFactory? You'd have to wrap the factory and the listener itself, but that doesn't seem too bad. This solution is already specific to the SocketTransportFactory anyway. UseListenerFilter should work with any transport.

[kevin-montrose]

@halter73 unfortunately, wrapping SocketTransportFactory isn't sufficient.

The closest we can get to the underlying Socket is the ConnectionContext returned from SocketConnectionListener.AcceptAsync(CancellationToken) - which is actually a SocketConnection, which has already been started. SocketConnection.Start() spins up async reads and writes on the underlying Socket, at which point we can no longer move the Socket. The move isn't possible both because some data might have come off of it (which might be surmountable), but also because a Socket's underlying handle cannot move between processes once an IOCP has been bound (on Windows anyway, see DuplicateAndClose).

[mconnew]

@kevin-montrose, I'm curious how you will peek at the TLS ClientHello? Do you intend to only do synchronous IO to peek at the TLS ClientHello? If the clients are slow to send data, this could be a scalability bottleneck.

[kevin-montrose]

@mconnew That is a concern in general but in our particular case we can rely on SNI already being seen and parsed upstream of our app (but still on our infrastructure [unfortunately, upstream cannot send to multiple ports/processes for us]).

If we could not rely on that, we could still peek synchronously and move Sockets with incomplete ClientHello records onto other threads or tasks and reroute (or return) them asynchronously - there's no requirement that Sockets be returned in the order they would "originally" be accepted.

[davidfowl]

I'm currently on the same page as @halter73. Maybe we can avoid doing any socket operations until the first call to ReadAsync/WriteAsync on the SocketConnection as a way to work around that. Would that be enough?

[kevin-montrose]

@davidfowl With that the Socket would still be movable but I wouldn't be able to get at it without reflection as SocketConnection is internal (and doesn't have an accessor for the Socket either).

Not calling ConnectionContext.Start() until the first read/write and making SocketConnection and TransportConnection (it's base class) public and adding an accessor for SocketConnection._socket would be sufficient. That strikes me as a lot of internal details to add to the public api, but I have no sense of how burdensome it'd be.

[davidfowl]

@davidfowl With that the Socket would still be movable but I wouldn't be able to get at it without reflection as SocketConnection is internal (and doesn't have an accessor for the Socket either).

The socket is exposed via the IConnectionSocketFeature.

That strikes me as a lot of internal details to add to the public api, but I have no sense of how burdensome it'd be.

It requires a bunch of wrapping.

[kevin-montrose]

@davidfowl ooo, I did not know IConnectionSocketFeature had been added.

With that, then yes if no async operations have happened on the Socket when SocketConnectionListener.AcceptAsync(...)'s returned task completes I'd be able to wrap SocketTransportFactory and get at what I need. I suppose such behavior would need to be contractual and have a test, since I'd be depending on it holding for all future releases.

That would be much cleaner than this random delegate.

[davidfowl]

Closing this in favor of #34458. The idea is that you should be able to write a connection middleware that runs first and grabs the IConnectionSocketFeature and parties on the socket before any read/write operations occur.