dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.98k stars 4.66k forks source link

UDP socket cannot be disconnected with Connect(IPAddress.Any, 0) #77962

Closed wasabii closed 1 year ago

wasabii commented 1 year ago

Description

It is fairly common (at least among those apps that do a lot of low level UDP stuff) to disconnect a UDP socket (revert SendTo target) by sending socket.EndConnect(socket.BeginConnect(new IPEndPoint(IPAddress.IPv6Any, 0), null, null)); This code works perfectly fine in Framework. It mirrors the same type of idea in BSD sockets and Winsock where you can simply disconnect the socket by connecting it to a AF_UNSPEC. Which is a constant to 0. Which for whatever reason .NET has been happy with IPAddress.Any.

It's also the documented operation on Linux, BSD, etc, for disconnecting a UDP socket (minus the weird .NET-async requirement).

Connectionless sockets may dissolve the association by connecting to an address with the sa_family member of sockaddr set to AF_UNSPEC (supported on Linux since kernel 2.2).

However, this doesn't appear to work in Core 6, as far as I can tell, because Core 6 has a check for IsConnected at the top of ConnectAsync (called from BeginConnect). Framework has no similar check (if reference source is to be believed).

I can't find a work around for this. Disconnect won't work, since that has a similar problem with isDisconnected.

Is this a bug? Or an undocumented change to the way Socket.Connect is suppsoed to function in Core? If so, how are you to disconnect a UDP socket but preserve it otherwise for reconnection?

Reproduction Steps

Create a DGRAM Socket. Call Connect(). Call connect a second time, with IPAddress.IPAny/0.

Expected behavior

I'd expect it to work like Framework, and BSD sockets, and WinSock.

Actual behavior

A request to send or receive data was disallowed because the socket is not connected and (when sending on a datagram socket using a sendto call) no address was supplied.

(SocketException with SocketError of NotConnected).

Regression?

Yes! Worked in Framework.

Known Workarounds

I wish.

Configuration

No response

Other information

No response

ghost commented 1 year ago

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

Issue Details
### Description It is fairly common (at least among those apps that do a lot of low level UDP stuff) to disconnect a UDP socket (revert SendTo target) by sending `socket.EndConnect(socket.BeginConnect(new IPEndPoint(IPAddress.IPv6Any, 0), null, null));` This code works perfectly fine in Framework. It mirrors the same type of idea in BSD sockets and Winsock where you can simply disconnect the socket by connecting it to INET_ANY. It's also the documented operation on Linux, BSD, etc, for disconnecting a UDP socket (minus the weird .NET-async requirement). However, this doesn't appear to work in Core 6, as far as I can tell, because Core 6 has a check for IsConnected at the top of `ConnectAsync` (called from BeginConnect). Framework has no similar check. I can't find a work around for this. Disconnect won't work, since that has a similar problem with isDisconnected. Is this a bug? Or an undocumented change to the way Socket.Connect is suppsoed to function in Core? If so, how are you to disconnect a UDP socket but preserve it otherwise for reconnection? ### Reproduction Steps Create a DGRAM Socket. Call Connect(). Call connect a second time, with IPAddress.IPAny/0. ### Expected behavior I'd expect it to work like Framework, and BSD sockets, and WinSock. ### Actual behavior A request to send or receive data was disallowed because the socket is not connected and (when sending on a datagram socket using a sendto call) no address was supplied. (SocketException with SocketError of NotConnected). ### Regression? Yes! Worked in Framework. ### Known Workarounds I wish. ### Configuration _No response_ ### Other information _No response_
Author: wasabii
Assignees: -
Labels: `area-System.Net.Sockets`
Milestone: -
wasabii commented 1 year ago

I should note, it would be more appropriate for me to say "dissolve" instead of "disconnect" wherever I said the latter.

wfurt commented 1 year ago

Can you post some more complete application @wasabii? I'm not quite sure what you are trying to achieve. In general, we should mimic Framework unless there is good reason not to.

wasabii commented 1 year ago

Very simple reproduction I created:

        public static void Main(string[] args)
        {
            var s = new System.Net.Sockets.Socket(System.Net.Sockets.AddressFamily.InterNetworkV6, System.Net.Sockets.SocketType.Dgram, System.Net.Sockets.ProtocolType.Udp);
            s.Connect(new IPEndPoint(IPAddress.IPv6Loopback, 3231));
            s.Connect(new IPEndPoint(IPAddress.IPv6Any, 0));
            s.Connect(new IPEndPoint(IPAddress.IPv6Loopback, 3231));
            s.Connect(new IPEndPoint(IPAddress.IPv6Any, 0));
        }

This runs fine in .NET 48. It fails on the second Connect call on .NET 6 (it also fails on Core 3.1)

wasabii commented 1 year ago

For completeness, this also fails on .NET Core, but succeeds on .NET Framework.

        public static void Main(string[] args)
        {
            var s = new System.Net.Sockets.Socket(System.Net.Sockets.AddressFamily.InterNetworkV6, System.Net.Sockets.SocketType.Dgram, System.Net.Sockets.ProtocolType.Udp);
            s.EndConnect(s.BeginConnect(new IPEndPoint(IPAddress.IPv6Loopback, 3231), null, null));
            s.EndConnect(s.BeginConnect(new IPEndPoint(IPAddress.IPv6Any, 0), null, null));
            s.EndConnect(s.BeginConnect(new IPEndPoint(IPAddress.IPv6Loopback, 3231), null, null));
            s.EndConnect(s.BeginConnect(new IPEndPoint(IPAddress.IPv6Any, 0), null, null));
        }
wasabii commented 1 year ago

The cause is because .NET Core's Socket.cs has a check near the beginning for isConnected:

            if (_isConnected)
            {
                throw new SocketException((int)SocketError.IsConnected);
            }

Framework has no similar check. It allows the Connect call to continue, regardless of the isConnected state. This makes sense: as the isConnected state isn't particularly relevant for UDP work. Also, enforcement of such a thing really appropriately belongs to the native socket library itself.

wasabii commented 1 year ago

So, I worked around this. It's evil:

#if NETCOREAPP3_1_OR_GREATER
                    // HACK .NET Core has an explicit check for _isConnected https://github.com/dotnet/runtime/issues/77962
                    SocketIsConnectedFieldSetter(socket, false);
#endif
        static readonly FieldInfo SocketIsConnectedField = typeof(Socket).GetField("_isConnected", BindingFlags.NonPublic | BindingFlags.Instance);
        static readonly Action<Socket, bool> SocketIsConnectedFieldSetter = MakeFieldSetter<Socket, bool>(SocketIsConnectedField);

Literally using reflection to reset the _isConnected field. =(

wfurt commented 1 year ago

You can do something like

using System.Net.Sockets;
using System.Net;

{
    var s = new System.Net.Sockets.Socket(System.Net.Sockets.AddressFamily.InterNetworkV6, System.Net.Sockets.SocketType.Dgram, System.Net.Sockets.ProtocolType.Udp);
    s.Connect(new IPEndPoint(IPAddress.IPv6Loopback, 3231));
    s.Disconnect(reuseSocket: true);
    s.ConnectAsync(new IPEndPoint(IPAddress.IPv6Any, 0)).GetAwaiter().GetResult();
    s.Disconnect(reuseSocket: true);
    s.ConnectAsync(new IPEndPoint(IPAddress.IPv6Loopback, 3231)).GetAwaiter().GetResult();
    s.Disconnect(reuseSocket: true);
    s.ConnectAsync(new IPEndPoint(IPAddress.IPv6Any, 0)).GetAwaiter().GetResult();
}

I don't know why there is limit for Async but the tasks will finish synchronously for UDP so there no thread pool work in practice.

wasabii commented 1 year ago

@wfurt That actually does seem to work on Linux on NET 6. But breaks on Windows. Disconnect() throws an error on Windows. SocketError.NotConnected.

wasabii commented 1 year ago

Fail on Linux on .NET Core 3.1, too, with an InvalidOperationException on the second connect. On this one the error text is:

System.InvalidOperationException: Once the socket has been disconnected, you can only reconnect again asynchronously, and only to a different EndPoint. BeginConnect must be called on a thread that won't exit until the operation has been completed.

That's the general "you have to use async" error (which you just asked about). But Core 3.1 is emitting it even when you do async. So Disconnect on Core 3.1 is putting something in a bad state.

wfurt commented 1 year ago

3.1 is out of support in a month. I tested this on Ubuntu 20.04 with 6.0 and 7.0. AFAIK we could not disconnect UDP on macOS but that may be corner case. cc: @tmds for any more insight.

wasabii commented 1 year ago

Yeah. I'm going to need to keep making releases for 3.1 for the next few years I suspect, regardless of MS's support. So, yes, we found something that works on Linux Net 6, but fails on Windows Net 6 and fails on Net 48. Cleaner for my Core 6 binaries instead of the reflection, so that's good. But still have to conditionalize on everything else. Three of them at least.

I haven't even got to OS X yet, though that's on my list too. heh.

wfurt commented 1 year ago

It seems like the check was added explicitly to cover https://github.com/dotnet/runtime/issues/22978. Perhaps we should relax it for UDP or we should make Disconnect work consistently across platforms.

wasabii commented 1 year ago

Looking at #22978 there was definitely not enough research done there. They found it weird that it succeeded in the native libraries. Even though the man pages for connect explicitly mention it.

antonfirsov commented 1 year ago

Triage: this is something we can easily fix, we should do it. We can keep the exception for SocketException stream sockets, and forward the connect call otherwise. Compat with .NET Framework