Remove Client property from TcpClient and UdpClient

[stephentoub]

The property leaks through the abstraction. It's rarely used. And it complicates things on Unix due to exposing the underlying socket instance combined with difficulties with multiple connect calls on a single socket instance.

Until we have good reason to expose it in the contract, we're removing it from the contract before it's stable.

cc: @davidsh, @CIPop, @SidharthNabar, @terrajobst, @weshaggard, @saurabh500
Closes #4968
Closes #5411

[Maxwe11]

hm...how to get or set ReceiveTimeout/SendTimeout for UdpClient with these changes?

[stephentoub]

hm...how to get or set ReceiveTimeout/SendTimeout for UdpClient with these changes?

@CIPop, @davidsh?

[davidsh]

hm...how to get or set ReceiveTimeout/SendTimeout for UdpClient with these changes?

There are many granular properties of a Socket like ReceiveTimeout/SendTimeout that might be useful as direct properties of the TcpClient, UdpClient class. These can all be considered for a future version of the System.Net.Sockets contract. For devs that need this kind of low level control, they can create a Socket directly and set properties on it. The TcpClient, UdpClient class are merely higher level convenience classes.

[saurabh500]

@corivera PTAL

[corivera]

Looks good for SqlClient.

[CIPop]

LGTM

[stephentoub]

@davidsh, you can merge this at your convenience when you're ready to handle any internal test issues.

[davidsh]

@stephentoub

you can merge this at your convenience when you're ready to handle any internal test issues.

I've checked in the internal test changes. So, we can merge this when GREEN.

[davidsh]

@stephentoub Thx for doing the PR and driving related downstream changes.

[clairernovotny]

Here's a good reason we need this - I cannot currently create a UdpClient that shares a socket correctly. I get an exception with "Only one usage of each socket address (protocol/network address/port) is normally permitted".

The way I handled this before was to create a UdpClient with the default ctor, use the SetSocketOptions for sharing/non-exclusive use, then call Client.Bind(localEndpoint).

It's now impossible to do this because the ExclusiveAddressUse is a property that gets set after UdpClient is constructed.

[clairernovotny]

One option might be to put another ctor arg for exclusive address use so that we can pass that in up-front and prevent the issue?

[Maxwe11]

Milestone 1.1.0

Does it mean that in 1.0 RTM TcpClient will be shipped with Client property?

[davidsh]

Does it mean that in 1.0 RTM TcpClient will be shipped with Client property?

No. That was a mistake in Milestone labeling. This PR was merged a while ago.

You can look at the current System.Net.Sockets contract here and see what properties are available on the TcpClient class:
https://github.com/dotnet/corefx/blob/master/src/System.Net.Sockets/ref/System.Net.Sockets.cs

[clairernovotny]

Without either Client exposed on UdpClient or a way to set exclusive to false before the bind via the ctor, I need to use this really ugly hack along with a rd.xml file to deal with .net native. This is bad just to get it to work :)

static Socket GetSocketFromUdpClient(UdpClient client)
        {
            var mi = GetSocketMember.Value as MethodInfo;
            if (mi != null)
            {
                return (Socket)mi.Invoke(client, null);
            }
            var fi = GetSocketMember.Value as FieldInfo;
            if (fi != null)
            {
                return (Socket)fi.GetValue(client);
            }

            throw new NotSupportedException("Could not locate Socket");
        }

        static readonly Lazy<MemberInfo> GetSocketMember = new Lazy<MemberInfo>(GetSocketMemberImpl);

        static MemberInfo GetSocketMemberImpl()
        {
            // See if there's a client prop
            var pi = typeof(UdpClient).GetRuntimeProperties().FirstOrDefault(p => p.PropertyType == typeof(Socket));
            if (pi != null)
                return pi.GetMethod;

            var mi = typeof(UdpClient).GetRuntimeFields().First(fi => fi.FieldType == typeof(Socket));
            return mi;
        }
    }

[davidsh]

@onovotny I totally understand your requirements. However, it was not possible to leave the .Client property on the class as returning the raw socket caused problems when one tried to manipulate it later on especially on *Nix platforms.

Please open a new issue (since this PR is merged and closed) and propose an API change. We then all can discuss how to address this requirement. You can reference this PR in your new issue. Thx.

[clairernovotny]

Done: https://github.com/dotnet/corefx/issues/8552

[stephentoub]

However, it was not possible to leave the .Client property on the class as returning the raw socket caused problems when one tried to manipulate it later on especially on *Nix platforms.

To clarify, there were a few concerns with these properties:

• TcpClient.Client is problematic on Unix. BSD Sockets have undefined behavior after connect on a socket fails, which means we need to play tricks to implement a method like Connect with a DnsEndPoint, as that could actually end up needing to try to connect to multiple sites. We can achieve this by using a new Socket for each attempt, keeping the one that works, but that then conflicts with the Client property, which exposes the actual Socket being used, and potentially before Connect has been used. If we choose to expose this property (which would can certainly look at for post-1.0), we'll need to either make it throw prior to connecting, or prohibit connecting with a DNS endpoint / string host after the Client property has been accessed.
• To my knowledge, there isn't a similar issue with UdpClient.Client, other than consistency with TcpClient.
• TcpClient and UdpClient are extremely thin wrappers around a Socket, meant to provide a slightly higher level of abstraction, yet exposing the underlying Socket bleeds through that abstraction.

[clairernovotny]

@stephentoub Thanks for the clarification. While the UdpClient wrapper may be "thin," there's a lot of code in there. The biggest ones are around getting SendAsync/ReceiveAsync to work correctly...there's a lot of gunk in there....and I like the abstraction over using Socket's directly. My concern is really that I cannot do one thing with the UdpClient alone, but that's fixable.