Socket.Select throws ObjectDisposedException when one of the sockets is disposed

[drieseng]

In (desktop) .NET Framework, Socket.Select(....) throws a SocketException when one of the sockets that you specify is disposed:

System.Net.Sockets.SocketException (10038): An operation was attempted on something that is not a socket.

In .NET Core 2.1 and 3.1, this is also the exception that is thrown on Windows. On Linux, no exception is thrown.

Now in .NET 5.0 Preview 2, an ObjectDisposedException is thrown on both Windows and Linux:

System.ObjectDisposedException: Safe handle has been closed.
Object name: 'SafeHandle'.
at System.Runtime.InteropServices.SafeHandle.DangerousAddRef(Boolean& success)
at System.Net.Sockets.SocketPal.AddToPollArray(PollEvent* arr, Int32 arrLength, IList socketList, Int32& arrOffset, PollEvents events, Int32& refsAdded)
at System.Net.Sockets.SocketPal.SelectViaPoll(IList checkRead, Int32 checkReadInitialCount, IList checkWrite, Int32 checkWriteInitialCount, IList checkError, Int32 checkErrorInitialCount, PollEvent* events, Int32 eventsLength, Int32 microseconds)
at System.Net.Sockets.SocketPal.Select(IList checkRead, IList checkWrite, IList checkError, Int32 microseconds)
at System.Net.Sockets.Socket.Select(IList checkRead, IList checkWrite, IList checkError, Int32 microSeconds)

To reproduce, compile and run the following code fragment:

using System.Collections.Generic;
using System.Net;
using System.Net.Sockets;

class SelectDisposed
{
    private static void Main()
    {
        var endPoint = new IPEndPoint(IPAddress.Loopback, 8080);
        var client = new Socket(endPoint.AddressFamily, SocketType.Stream, ProtocolType.Tcp);
        client.Dispose();

        Socket.Select(new List<object> { client }, null, null, -1);
    }
}

[ghost]

Tagging @dotnet/ncl as an area owner. If you would like to be tagged for a label, please notify danmosemsft.

[tmds]

This is caused by dotnet/corefx#41763.

With the old behavior, no reference counting was performed. The SocketException refers to using a handle which is no longer a socket.
In a worse scenario, the handle could have been reused for a different Socket causing the Select to be performed against the wrong socket.

With the change, Select will require the handle to be valid. If the handle is not valid ODE is thrown.

[karelz]

So it is technical breaking change in 5.0.
We should either document it as such, or wrap the ObjectDisposedException in SocketException.

@drieseng how did you discover the difference? Did you run into it in your app?

[stephentoub]

So it is technical breaking change in 5.0. We should either document it as such

It's fine to document it. It's a bug that should have been fixed years ago when things were moved to be SafeHandle-based. It's a handle-recycling issue.

[karelz]

OK, marking as such. cc @antonfirsov

[drieseng]

I'm really impressed by the speed at which this decision was taken and verified across all implementations (.NET, Mono, ...). Because this is how such a thing is done, right ?

Note that I'm ok with this decision, but I would expect there to be more discussion on such changes (even if you can consider it an implementation detail). In the desktop framework, there was a very high level of compatibility and a slower pace of innovation. I definitely welcome the high pace of innovation that we have right now, but I get the impression that compatibility is sometimes abandoned all too fast. This specific issue is perhaps not the best example of this, but it does show easy "breaking" changes are introduced.

[danmoseley]

It is certainly nice that the test is running on Mono automatically. It shows the value of all being in one repo.

[drieseng]

That's indeed true.

[karelz]

Fixed by PR dotnet/dotnet-api-docs#4720