-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Description
The normal pattern to listen for incoming connections is:
using (var s = new Socket(...)) {
s.Bind(...);
s.Listen(...);
while (true) {
var connectionSocket = s.Accept();
Task.Run(() => ProcessConnection(connectionSocket));
}
}The only way to reliably cancel that loop is to close the socket. This works, but it causes an ObjectDisposedException which is usually one of the reserved exception types that always indicate a bug such as NRE and ArgumentNullException. Catching that exception might hide real bugs. You might confuse the intended ObjectDisposedException with one that is not supposed to happen but did (bug).
Also, this seems to be racy. What if the handle is closed right before Accept passes it to the native accept function? Is that even thread-safe?! Could be a use of an invalid/unintended handle. Note sure if the BCL has a way to prevent that scenario.
Some people suggest to break the loop by connecting to that socket. That might not be possible depending on what interface you are listening on. There are no guarantees to be able to connect anywhere at all. Also, this feels like such a dirty hack.
There are ideas of using IAsyncResult.WaitHandle with a timeout or in combination with another wait handle but that's quite nasty code and much slower than not using a wait handle. It's much faster to let the kernel wait as part of a synchronous call than to execute an asynchronous call, then create an event object, wait for it and then dispose of all of this. Also, in case you do a WaitHandle.WaitAny(ioHandle, cancelHandle) and you find yourself in the cancellation case you still need to abort the IO using Socket.Close which is the same dirtiness.
It does not matter for this issue whether you're using sync or async IO.
Please add a way to cancel any long-running socket operation such as accept, connect, read, write, shutdown in a clean way. This could take the form of a CancellationToken or a method Socket.AbortPendingOperations(). A cancelled operation should throw an exception that allows to identify the situation so that it can be caught reliably.
Cancelling is not just important for breaking the accept loop but it's useful for all other long-running operations as well. Sometimes you need to be able to shut down processing.