AsyncEx icon indicating copy to clipboard operation
AsyncEx copied to clipboard

AsyncContextThread dies on exception thrown inside async void method

Open ekalchev opened this issue 6 years ago • 3 comments

This below reproduce the issue with v5.0.0. AsyncContexstThread is destroyed when you throw exception from async void method.

Note that if you remove the async keyword it works fine. This is problem when you use async void event handlers

using Nito.AsyncEx;
using System;
using System.Diagnostics;
using System.Threading;
using System.Threading.Tasks;

namespace ConsoleApp3
{
    class Program
    {
        static void Main(string[] args)
        {
            var test = new Test();
            test.Run().Wait();
        }
    }

    class TestClassWithEvent
    {
        public async void Throw()
        {
            throw new NotImplementedException();
        }
    }

    class Test
    {
        public async Task Run()
        {
            AsyncContextThread outerThread = new AsyncContextThread();
            var testObj = new TestClassWithEvent();

            try
            {
                await outerThread.Factory.StartNew(() =>
                {
                    Thread.CurrentThread.Name = "test thread"; // <-- thread is destroyed
                    testObj.Throw();
                });
            }
            catch { }

            await outerThread.Factory.StartNew(() =>
            {
                var a = 1; // add breakpoint here - this is never called
            });

            await Task.Delay(300000);
        }
    }
}

ekalchev avatar Jul 10 '19 09:07 ekalchev

I'm closing this as expected behavior. An async void method surfaces its exceptions by throwing them directly on the SynchronizationContext, by design. In this sense, they are "top level" exceptions.

Some systems have "top level" exception handlers; AsyncContextThread does not. AsyncContextThread interprets a top-level exception as a conclusion of that main loop.

If you disagree with the behavior being "expected", then feel free to re-open with a description of what behavior you do expect, in particular what should be done with the exception.

StephenCleary avatar Jul 18 '19 03:07 StephenCleary

I'll give you one use case that this is a big problem:

You are writing base functionality that other users should use. AsyncContextThread is implementation details that is not visible to other developers and they are not aware if their code will be run on AsyncContextThread or ThreadPool.

if any developer write async void method that throws exception that will kill the thread of the underlying infrastructure and any other code scheduled to run on that thread will not run. You as an author of that base functionality has absolutely no control to prevent that. Even if you are the only developer working with this code you can't guarantee in the future you won't write async void event handler that can "leak" exceptions.

I believe there should be event on the AsyncContext or AsyncContextThread which will give you a chance to observe those exceptions similar to TaskScheduler.UnobservedTaskException and prevent thread exit.

Currently the thread dies, you can still queue tasks which will never be executed and there is no indication that something went wrong. At least AsyncContextThread should go in invalid state and throw exception if you attempt to queue task in this state.

ekalchev avatar Jul 18 '19 06:07 ekalchev

@StephenCleary How to reopen this?

ekalchev avatar Jul 30 '19 16:07 ekalchev