Socket.Connect methods that connect to multiple endpoints are broken on Unix

[ericeil]

Unlike Windows, the sockets API on Linux and OSX does not allow a single socket to attempt more than one connection. We had attempted to do this on Linux, but it did not work (see #16238).

Socket has multiple methods that depend on being able to try connections to multiple endpoints on a single socket:

public void Connect(IPAddress[] addresses, int port)

...attempts to connect to each of addresses, returning when one of them is successful.

public void Connect(string host, int port)

...does a DNS lookup, which may return multiple addresses, then attempts to connect to each of them until successful.

public void Connect(EndPoint remoteEP)

If this is passed a DnsEndPoint, then this behaves the same as Connect(string, int), trying all resolved addresses.

Additionally, we support async variants of all of these.

On Unix we currently "fake" this by creating multiple temporary sockets behind the scenes, trying to connect each of them to a different address. If one succeeds, we immediately disconnect it and connect the "real" socket to that address. This workaround has some possibly unacceptable implications:

1. Whichever endpoint we finally choose, we will end up connecting to it twice, once for the "probe" connection, to see if it will work, and again for the "real" connection. This will cause issues for servers that do not expect more than one connection, for load balancers that try to distribute connections, etc. It's also a lot of undesired network overhead.

2. Even though the first connection attempt succeeded, the second will not necessarily succeed. This is especially true in scenarios like DNS-based load balancing. So we have a functional issue here, where we may fail to connect the "real" socket in cases where we would have succeeded.

I do not think we should leave this as-is, as it will create too many unexpected strange behaviors in code that uses these, expecting them to behave the same way as they do on Windows. So I propose doing one of the following:

a) Remove the Connect overloads that take multiple endpoints, and the DnsEndpoint type, from the public contracts.

-- or --

b) Change these Connect overloads to throw PlatformNotSupportedException on Unix, Also, change Connect(EndPoint, int) to throw PlatformNotSupportedException if passed a DnsEndPoint.

In either case, we should provide sample code for how to code equivalent behavior using the remaining APIs.

@stephentoub, @pgavlin, @CIPop, @davidsh, @SidharthNabar, please provide feedback.

[pgavlin]

I feel like I should also point out another option: we could replace all of the instance Connect methods with static equivalents that return a connected Socket instance. This design would be implementable on any POSIX-compliant sockets implementation and would not imply the removal of DnsEndpoint. Unfortunately, it has the distinct downside of dramatically changing the surface area of the S.N.Sockets contract.

I'm personally in favor of option (b) with the possible addition of a toolable notification that the existing APIs should not be used for cross-platform code (e.g. a Roslyn diagnostic or some appropriate attribute).

[davidsh]

b) Change these Connect overloads to throw PlatformNotSupportedException on Unix, Also, change Connect(EndPoint, int) to throw PlatformNotSupportedException if passed a DnsEndPoint.

I would vote for b).

[stephentoub]

Sounds like we should go for (b), ideally where the message in the PlatformNotSupportedException provides some guidance.

[bgrainger]

I just tried to run a .NET Core app (that uses sockets) on Ubuntu 14.04. Calling Socket.Connect(hostname, port) threw a PlatformNotSupportedException : Sockets on this platform are invalid for use after a failed connection attempt, and as a result do not support attempts to connect to multiple endpoints.

I did not find this exception message to be helpful in explaining the underlying issue or advising how to avoid it. At first I thought the message meant that I was attempting to reuse a socket after a failed connection attempt (which didn't make sense). Eventually I found 30bd4b7 and (thanks to the very helpful commit message) understood the problem (and the workaround).

I propose two improvements:

• Rewrite the exception message to clearly indicate the specific problem and ideally a workaround: e.g., "Connect(string) and Connect(IPAddress[]) are not supported on this platform; use Connect(IPAddress) or Connect(EndPoint) instead", or "This overload of (Begin)Connect is not supported on this platform; see aka.ms/... for more information" (which could link to an issue or wiki page here?).
• Mark the non-supported Connect overloads as [Obsolete], even for Windows builds. If I'm writing a library that I intend to be cross-platform, I'd like notice as soon as possible that the code I'm writing will not work on other platforms, especially if I'm typically building and testing on Windows.

I'd be happy to open a PR for either/both of these suggestions if you're in favour of them.

[stephentoub]

@bgrainger, sorry the error message wasn't as helpful as it could have been (but I'm glad the commit message was). You're welcome to submit a PR to improve it. As for marking the overloads as [Obsolete], that's a question for @davidsh or @CIPop.

[NickCraver]

@stephentoub Was the intent here really to disable connecting to sockets via hostname on some platforms? Option 2 in that commit rather needlessly breaks StackExchange.Redis on OS X for example, even when trying to connect to a single endpoint (as we always do).

We just got this issue filed: StackExchange/StackExchange.Redis#410 All of the socket code I've written goes to a single host and that's all broken cross-platform now, without merit I think.

IMO, we can and should break far less code with Option 3 and a clearer commit message here. There's no need to break people aside from erroring on what would actually break.

[stephentoub]

@NickCraver, if you open a new issue to @ericeil, we can reevaluate.

[NickCraver]

@stephentoub k, will do that when time allows today - thanks.