dotnet / runtime

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

Add method to avoid TaskScheduler.UnobservedTaskException for a given Task #23878

Closed ReubenBond closed 1 year ago

ReubenBond commented 6 years ago

Unobserved Task exceptions are propagated to the TaskScheduler.UnobservedTaskException event. There are cases where we want to be able to ignore the results of a task without triggering that event.

For example, we have an extension method, task.WithTimeout(TimeSpan) which will throw a TimeoutException if a task doesn't complete before a specified time. In this case we want to ignore the results of the original task.

Currently we use an extension method to ignore individual tasks:

public static void Ignore(this Task task)
{
    if (task.IsCompleted)
    {
        var ignored = task.Exception;
    }
    else
    {
        IgnoreAsync(task);
    }

    async void IgnoreAsync(Task asyncTask)
    {
        try
        {
            await asyncTask.ConfigureAwait(false);
        }
        catch
        {
            // Ignored.
        }
    }
}

That method still has a cost, though, and ideally we would like to remove most of that cost. A method inside the framework to ignore the results of a task without needing to register/allocate a continuation would be beneficial here.

baal2000 commented 2 years ago

Another suggestion is to improve the framework so it helps developers troubleshoot the abandoned tasks failures. The failing thread call stack without the context of the async execution flow may not always be useful. As in the following example of such an event:

System.AggregateException: A Task's exception(s) were not observed either by Waiting on the Task or accessing its Exception property. As a result, the unobserved exception was rethrown by the finalizer thread. (The operation has timed out.)
---> System.TimeoutException: The operation has timed out.
--- End of inner exception stack trace ---
stephentoub commented 2 years ago

Another suggestion is to improve the framework so it helps developers troubleshoot the abandoned tasks failures. The failing thread call stack without the context of the async execution flow may not always be useful.

What specifically do you suggest be done? Taking a snapshot of the stack when every task is created would be utterly painful, even in debug.

baal2000 commented 2 years ago

Taking a snapshot of the stack when every task is created would be utterly painful, even in debug.

I understand, but if it could be necessary in specific cases. For instance a static flag/environment variable could be used to enable that for testing/troubleshooting purposes only when the performance hit can be tolerated.

We do have an internal purpose class named RecyclableList that allocates arrays off an ArrayPool. The recommended practice is to dispose of every instance of it before dereferencing to return the array into the pool. The finalizer fires an event (similar to the Tasks) if it finds out an instance is not disposed of, and provides the initialization callstack.

I think there is a real need for something similar to be done for Tasks.

stephentoub commented 1 year ago

A method inside the framework to ignore the results of a task without needing to register/allocate a continuation would be beneficial here.

Is that something we want to encourage, though? We certainly have places in dotnet/runtime, for example, where we want to "ignore" such exceptions, but ignoring them typically doesn't mean dropping them entirely, but rather typically logging that they occurred in some way. For example, System.Net.Http has this: https://github.com/dotnet/runtime/blob/b5fb7d002acbf729151da9d9918a93c8261d1e95/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs#L90-L129 Indeed, one of those is just such an Ignore method like that suggested, but it's used literally once in the whole code base, whereas the corresponding Log method that suppresses the exception but also traces it out is used ~20 times.

I'm tempted to say that we should encourage developers to use await/ContinueWith to implement their own such tracing/logging for failures, rather than making it easy and enticing to ignore exceptions entirely.

Does Orleans still use its own Ignore here? It doesn't have any need to log such failures in any way?

ReubenBond commented 1 year ago

Yes, we still have an Ignore() method, currently defined like so:

private static readonly Action<Task> IgnoreTaskContinuation = t => { _ = t.Exception; };

public static void Ignore(this Task task)
{
    if (task.IsCompleted)
    {
        _ = task.Exception;
    }
    else
    {
        task.ContinueWith(
            IgnoreTaskContinuation,
            CancellationToken.None,
            TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously,
            TaskScheduler.Default);
    }
}

Looking at the current usages, they are mostly cases where we are messaging another host in a fire-and-forget manner (eg, gossip or other notifications). The other dominant case is background method calls where the outcome is inconsequential at that particular call site. In our case, some of the methods will perform their own logging before rethrowing.

I don't feel strongly about this issue and would be just as happy were it to be closed.

stephentoub commented 1 year ago

Ok, thanks for the details, that makes sense. I'm going to close it.

p.s. "gossip" is a fun name :)