StephenCleary / AsyncEx

A helper library for async/await.
MIT License
3.49k stars 358 forks source link

Suggestion - "safe" WaitAsync #236

Closed yfital closed 2 years ago

yfital commented 3 years ago

Hey,

Currently, we use the WaitAsync for various external methods which do not support CT. It is great from the client POV, but it still poses an unobserved threat.

A simple example of the problem:


internal static async Task Main(string[] args)
        {
            AppDomain.CurrentDomain.UnhandledException += (sender, eventArgs) =>
            {
                Console.WriteLine($"UnhandledException {eventArgs.ExceptionObject as Exception}");
            };

            TaskScheduler.UnobservedTaskException += (sender, eventArgs) =>
            {
                Console.WriteLine($"UnobservedTaskException {eventArgs.Exception}");
            };

            using var cts = new CancellationTokenSource(1000);

            try
            {
                var a = await MyAsyncFunction().WaitAsync(cts.Token);
            }
            catch (Exception e)
            {
                Console.WriteLine($"Catch {e}");
            }

            while (true)
            {
                GC.Collect();
                await Task.Delay(100);
            }

            Console.ReadLine();
        }

        public static async Task<int> MyAsyncFunction()
        {
            await Task.Delay(10000);
            throw new InvalidOperationException("blabla");
            return 10;
        }

    }

Running the above will result in:

  1. "Catch" exception
  2. "Unobserved" exception

Suggestion:


        public static async Task<T> ProperWaitAsync<T>(this Task<T> task, CancellationToken ct)
        {
            return await await task
                .ContinueWith(t =>
                {
                    if (t.IsFaulted && t.Exception is not null)
                        Console.WriteLine($"proper exception {t.Exception}");
                    return t;
                }).WaitAsync(ct);
        }

        public static Task<T> ProperWaitAsync2<T>(this Task<T> task, CancellationToken ct)
        {
            return task
                .ContinueWith(t =>
                {
                    if (t.IsFaulted && t.Exception is not null)
                        Console.WriteLine($"proper exception {t.Exception}");

                    return t;
                }).Unwrap().WaitAsync(ct);
        }

I'm leaning toward the first solution, but they are more or less equal. Of course, instead of console write line, it should be an OnError lambda.

If approved, I'll open a PR.

StephenCleary commented 2 years ago

The current behavior is as-designed. It has the same behavior as the new Task.WaitAsync method in .NET 6.

By canceling the wait, you do end up with a possible unobserved exception, so having it go through the normal UnobservedTaskException is expected.