1

I have recently discovered by accident that I don't need to await a Task to get the exceptions that Task throws for program control logic.

Pre discovery of this feature, my code would look like this:

using (TcpClient client = new TcpClient())
{
    try
    {
        var ca = client.ConnectAsync("192.168.1.88", 9999);
        await await Task.WhenAny(ca, Task.Delay(5000));
        if (ca.IsCompleted)
            Console.WriteLine("Connected");
        else
            Console.WriteLine("Connection failed due to timeout");
    }
    catch
    {
        Console.WriteLine("Connection failed.");
    }
}

Post discovery my code looks like this, (which personally I find more readable)

using (TcpClient client = new TcpClient())
{
    var ca = client.ConnectAsync("192.168.1.88", 9999);
    await Task.WhenAny(ca, Task.Delay(5000));
    if (ca.IsFaulted || !ca.IsCompleted)
        Console.WriteLine("Connection failed due to timeout or exception");
    else
        Console.WriteLine("Connected");
}

Is there anything technically wrong with intentionally leaving the unobserved exceptions to be ignored by the TaskScheduler? E.g., will they back up and cause other problems in the async/await framework?

The behavior is further described in this question.

Rowan Smith
  • 1,815
  • 15
  • 29
  • why do you await twice – Clint Feb 01 '20 at 07:33
  • Task.WhenAny returns a Task Awaiting the ``T`` Task causes the exceptions to be thrown if it's faulted. – Rowan Smith Feb 01 '20 at 07:39
  • @Clint https://stackoverflow.com/questions/12678013/task-whenany-and-unobserved-exceptions#comment82611521_12678434 – GSerg Feb 01 '20 at 08:04
  • 3
    @RowanSmith I believe the "not observing exceptions" part is well explained in the accepted answer of the question you are linking to. The only thing I would change about your code is checking which task is returned from `WhenAny` instead of going directly to checking `ca`. Depending on the environment and the way the tasks were started, the `if (ca.IsFaulted || !ca.IsCompleted)` may be a race condition provided that it was not `ca` that was returned from `WhenAny`. – GSerg Feb 01 '20 at 08:08
  • 1
    Leaving a faulted task unobserved used to mean it ends up being rethrown in the GC, unless the global event is hooked for unobserved tasks. This adds overhead even if it doesn't now kill your process. I don't know if that is still the case, but: What you're doing is a pretty horrible way of avoiding an exception. Catch is the correct and appropriate / supported mechanism. Frankly, I can't advise doing anything other than the direct catch. – Marc Gravell Feb 01 '20 at 08:13
  • 1
    @MarcGravell But even with a `catch` in place, it will only catch exception from the first task. Exceptions from the other tasks will remain unobserved, and if it's okay to leave them that way, then it's not clear why the exception from the winning task would be any different. – GSerg Feb 01 '20 at 08:16
  • 2
    @GSerg oh yes. I hadn't realised that both ways leave unobserved tasks. They are both functionally the same in the event ``Task.Delay()`` finishes first. – Rowan Smith Feb 01 '20 at 08:20

1 Answers1

2

E.g., will they back up and cause other problems in the async/await framework?

No. Discarding a task will not cause any "back up" issues like that. In fact, most usages of Task.WhenAny will result in a task being discarded.

As Marc pointed out in the comments, this approach does have some additional overhead: the task needs to be finalized and the exception is re-raised on a global event. So it is inefficient.

Is there anything technically wrong with intentionally leaving the unobserved exceptions to be ignored by the TaskScheduler?

As GSerg pointed out in the comments, there is a race condition in the code. Since the code doesn't actually use the "which task completed" result of WhenAny, it may time out and then the connection completes, and the code would take the Connected path. So I'd say this race condition is benign. But if you use this pattern elsewhere, think hard about the race conditions in that code, and whether they're acceptable.

There are some other inefficiencies in the code as well (in addition to the finalizer / unhandled exception event handler mentioned by Marc):

  • On a timeout, the client will still be trying to connect. To cancel the connection itself, close the socket.
  • When the socket connects, the timeout timer will still be running. Consider canceling the timer when the socket connects - e.g., by making the delay infinite but with a timed cancellation token.

If this is a desktop/console app, then you may be able to ignore these kinds of inefficiencies. If this is a server app, you'd probably want to tighten it all up.

TL;DR: The code would work, but has some inefficiencies and a race condition (benign in this case). So this pattern would only be appropriate in some application situations. That's why you don't see this kind of pattern a lot.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810