8

I have several asynchronous network operations that return a task that may never finish:

  1. UdpClient.ReceiveAsync doesn't accept a CancellationToken
  2. TcpClient.GetStream returns a NetworkStream that doesn't respect the CancellationToken on Stream.ReadAsync (checking for cancellation only at the start of the operation)

Both wait for a message that may never come (because of packet loss or no response for example). That means I have phantom tasks that never complete, continuations that will never run and used sockets on hold. I know i can use TimeoutAfter, but that will only fix the continuation problem.

So what am I supposed to do?

i3arnon
  • 113,022
  • 33
  • 324
  • 344
  • At least, TcpClient will throw an exception (or return 0 in stream.Read) eventually. so there won't be any *phantom* tasks. Assuming you create only one `UdpClient` in your application one *phantom* task isn't a problem. If you think i am wrong please post a real case(+code) so that we can make concrete answers. – L.B Jan 30 '14 at 22:45
  • @L.B It won't in `stream.ReadAsync` (added to the question). And i use both tcp and udp concurrently hundreds of times a second. – i3arnon Jan 30 '14 at 22:51
  • Sorry but I have written millions of codes using TcpClient or UdpClient but i never needed a method like this (of course in services running 24/7). BTW: `packet loss` doesn't apply to TCP – L.B Jan 30 '14 at 22:53
  • I assure you i didn't invent my situation. I ran out of udp ports while performance testing. – i3arnon Jan 30 '14 at 23:00
  • I guess it is because you don't know the internals of protocols and do not use the classes properly. I think no need to continue this discussion when there is no real case to talk about. Anyway, If it solves your problem than it is good enough. – L.B Jan 30 '14 at 23:04
  • 2
    @L.B, packet loss does apply to TCP, but it's handled transparently to the application layer. – Nathan Ernst Jan 31 '14 at 02:16
  • @NathanErnst Of course, everybody knows this. What happens when it is unrecovarable is explained in my first comment's first sentense. – L.B Jan 31 '14 at 10:44
  • @L.B that sentence is not relevant if you use `async-await`. – i3arnon Jan 31 '14 at 12:42

2 Answers2

10

So i've made an extension method on IDisposable that creates a CancellationToken that disposes the connection on timeout, so the task finishes and everything carries on:

public static IDisposable CreateTimeoutScope(this IDisposable disposable, TimeSpan timeSpan)
{
    var cancellationTokenSource = new CancellationTokenSource(timeSpan);
    var cancellationTokenRegistration = cancellationTokenSource.Token.Register(disposable.Dispose);
    return new DisposableScope(
        () =>
        {
            cancellationTokenRegistration.Dispose();
            cancellationTokenSource.Dispose();
            disposable.Dispose();
        });
}

And the usage is extremely simple:

try
{
    var client = new UdpClient();
    using (client.CreateTimeoutScope(TimeSpan.FromSeconds(2)))
    {
        var result = await client.ReceiveAsync();
        // Handle result
    }
}
catch (ObjectDisposedException)
{
    return null;
}

Extra Info:

public sealed class DisposableScope : IDisposable
{
    private readonly Action _closeScopeAction;
    public DisposableScope(Action closeScopeAction)
    {
        _closeScopeAction = closeScopeAction;
    }
    public void Dispose()
    {
        _closeScopeAction();
    }
}
i3arnon
  • 113,022
  • 33
  • 324
  • 344
  • Elegant hac...uh...solution! It never occurred to me to dispose the offending bad actor. Disruptive, but very effective! I suppose it would be pointless to try/catch the _closeScopeAction (my first inclination when providing an action-based API). I guess anything that barfs there should be exposed in a most obvious way as soon as possible. – Clay Sep 09 '15 at 17:36
  • @Clay I agree. If it can't actually handle the exception it shouldn't catch it. You may also want to look at this related question: [Is it safe to call BeginXXX without calling EndXXX if the instance is already disposed](http://stackoverflow.com/a/30226894/885318) – i3arnon Sep 10 '15 at 08:18
  • @Blue It may. Dispose should be able to be called twice. – i3arnon Jun 19 '22 at 19:52
3

So what am I supposed to do?

In this particular case, I would rather use UdpClient.Client.ReceiveTimeout and TcpClient.ReceiveTimeout to time out a UDP or TCP receive operation gracefully. I'd like to have the time-out error coming from the socket, rather than from any external source.

If in addition to that I need to observe some other cancellation event, like a UI button click, I would just use WithCancellation from Stephen Toub's "How do I cancel non-cancelable async operations?", like this:

using (var client = new UdpClient())
{
    UdpClient.Client.ReceiveTimeout = 2000;

    var result = await client.ReceiveAsync().WithCancellation(userToken);
    // ...
}

To address the comment, in case ReceiveTimeout has no effect on ReceiveAsync, I'd still use WithCancellation:

using (var client = new UdpClient())
using (var cts = CancellationTokenSource.CreateLinkedTokenSource(userToken))
{
    UdpClient.Client.ReceiveTimeout = 2000;
    cts.CancelAfter(2000);

    var result = await client.ReceiveAsync().WithCancellation(cts.Token);
    // ...
}

IMO, this more clearly shows my intentions as a developer and is more readable to a 3rd party. Also, I don't need to catch ObjectDisposedException exeception. I still need to observe OperationCanceledException somewhere in my client code which calls this, but I'd be doing that anyway. OperationCanceledException usually stands out from other exceptions, and I have an option to check OperationCanceledException.CancellationToken to observe the reason for cancellation.

Other than that, there's not much difference from @I3arnon's answer. I just don't feel like I need another pattern for this, as I already have WithCancellation at my disposal.

To further address the comments:

  • I'd only be catching OperationCanceledException in the client code, i.e.:

async void Button_Click(sender o, EventArgs args) { try { await DoSocketStuffAsync(_userCancellationToken.Token); } catch (Exception ex) { while (ex is AggregateException) ex = ex.InnerException; if (ex is OperationCanceledException) return; // ignore if cancelled // report otherwise MessageBox.Show(ex.Message); } }
  • Yes, I'll be using WithCancellation with each ReadAsync call and I like that fact, for the following reasons. Firstly, I can create an extension ReceiveAsyncWithToken:

public static class UdpClientExt
{
    public static Task<UdpReceiveResult> ReceiveAsyncWithToken(
        this UdpClient client, CancellationToken token)
    {
        return client.ReceiveAsync().WithCancellation(token);
    }
}

Secondly, in 3yrs from now I may be reviewing this code for .NET 6.0. By then, Microsoft may have a new API, UdpClient.ReceiveAsyncWithTimeout. In my case, I'll simply replace ReceiveAsyncWithToken(token) or ReceiveAsync().WithCancellation(token) with ReceiveAsyncWithTimeout(timeout, userToken). It would not be so obvious to deal with CreateTimeoutScope.

nawfal
  • 70,104
  • 56
  • 326
  • 368
noseratio
  • 59,932
  • 34
  • 208
  • 486
  • Timeouts are only relevant to synchronous operations, not async ones: "This option applies to synchronous Receive calls only. If the time-out period is exceeded, the Receive method will throw a SocketException." from http://msdn.microsoft.com/en-us/library/system.net.sockets.socket.receivetimeout(v=vs.110).aspx – i3arnon Jan 31 '14 at 00:13
  • @l3arnon, I updated the answer to address your comment. – noseratio Jan 31 '14 at 00:21
  • 1
    `WithCancellation` though won't really cancel the `ReceiveAsync` *phantom* task, it only allows to "abandon" it, just like `TimeoutAfter` (also by toub) – i3arnon Jan 31 '14 at 00:25
  • 1
    @l3arnon, it will cancel in it the same way your `DisposableScope` will: `client.Dispose` will be called as soon as the corresponding `using` scope ends. Which will end as soon as `WithCancellation` throws a `TaskCancelledException`. This way, I don't have to observe `ObjectDisposedException` as you do. – noseratio Jan 31 '14 at 00:28
  • You would need to observe `TaskCancelledException` though, and then.. what's really the difference between our answers. – i3arnon Jan 31 '14 at 00:31
  • btw.. That's what i used to do, but i thought clearly stating that im calling dispose is clearer than it being implied by closing the using scope. – i3arnon Jan 31 '14 at 00:34
  • @I3arnon, I moved my last comment to the answer. – noseratio Jan 31 '14 at 00:56
  • Last 2 things: 1. I get why you would like CanceledException, but not why you would be catching it anyway (I don't). 2. Your solution means that you need to add `WithCancellation` to every single async call in that scope (for example `tcpClient.ConnectAsync`, `SendAsync`, `ReceiveAsync`). – i3arnon Jan 31 '14 at 01:17
  • @I3arnon, if you're still interested, I further updated the answer to address your latest comments. – noseratio Jan 31 '14 at 01:42
  • 3
    I laughed at the ".NET 6.0" part. Well said. One should hope that by then Microsoft don't bring back the old UnobservedTaskException behaviour, because then a lot of people using `WithCancellation` (and not handling the exceptions thrown inside the abandoned task, i.e. `ObjectDisposedException` in this case) will be kicking themselves. – Kirill Shlenskiy Jan 31 '14 at 01:44