Avoid unobserved task exception in AsyncAtomicFactory (#686)

* AsyncAtomicFactory.Initializer: do not call SetException on TCS to avoid unobserved task exception when garbage collecting TCS

* Consider other exception type on build server

* Remove second GetValueAsync call

* ScopedAsyncAtomicFactory.Initializer: avoid unobserved task exception as well
Remove test in AsyncAtomicFactoryTests again

* fix

* task => thread

* Rename test in ScopedAsyncAtomicFactoryTests and log unobserved task exception to test output.

---------

Author: advdotnet

BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs

@@ -4,11 +4,19 @@
 using BitFaster.Caching.Atomic;
 using FluentAssertions;
 using Xunit;
+using Xunit.Abstractions;
 
 namespace BitFaster.Caching.UnitTests.Atomic
 {
     public class AsyncAtomicFactoryTests
     {
+        private readonly ITestOutputHelper outputHelper;
+
+        public AsyncAtomicFactoryTests(ITestOutputHelper outputHelper)
+        {
+            this.outputHelper = outputHelper;
+        }
+
         [Fact]
         public void DefaultCtorValueIsNotCreated()
         {
@@ -156,6 +164,46 @@ await Task.WhenAll(first, second)
             }
         }
 
+        [Fact]
+        public async Task WhenValueCreateThrowsDoesNotCauseUnobservedTaskException()
+        {
+            bool unobservedExceptionThrown = false;
+            TaskScheduler.UnobservedTaskException += OnUnobservedTaskException;
+
+            try
+            {
+                await AsyncAtomicFactoryGetValueAsync();
+
+                GC.Collect();
+                GC.WaitForPendingFinalizers();
+            }
+            finally
+            {
+                TaskScheduler.UnobservedTaskException -= OnUnobservedTaskException;
+            }
+
+            unobservedExceptionThrown.Should().BeFalse();
+
+            void OnUnobservedTaskException(object sender, UnobservedTaskExceptionEventArgs e)
+            {
+                outputHelper.WriteLine($"Unobserved task exception {e.Exception}");
+                unobservedExceptionThrown = true;
+                e.SetObserved();
+            }
+
+            static async Task AsyncAtomicFactoryGetValueAsync()
+            {
+                var a = new AsyncAtomicFactory<int, int>();
+                try
+                {
+                    _ = await a.GetValueAsync(12, i => throw new ArithmeticException());
+                }
+                catch (ArithmeticException)
+                {
+                }
+            }
+        }
+
         [Fact]
         public void WhenValueNotCreatedHashCodeIsZero()
         {

BitFaster.Caching.UnitTests/Atomic/ScopedAsyncAtomicFactoryTests.cs

@@ -4,11 +4,19 @@
 using BitFaster.Caching.Atomic;
 using FluentAssertions;
 using Xunit;
+using Xunit.Abstractions;
 
 namespace BitFaster.Caching.UnitTests.Atomic
 {
     public class ScopedAsyncAtomicFactoryTests
     {
+        private readonly ITestOutputHelper outputHelper;
+
+        public ScopedAsyncAtomicFactoryTests(ITestOutputHelper outputHelper)
+        {
+            this.outputHelper = outputHelper;
+        }
+
         [Fact]
         public void WhenScopeIsNotCreatedScopeIfCreatedReturnsNull()
         {
@@ -105,7 +113,7 @@ public void WhenValueIsCreatedDisposeDisposesValue()
         {
             var holder = new IntHolder() { actualNumber = 2 };
             var atomicFactory = new ScopedAsyncAtomicFactory<int, IntHolder>(holder);
-            
+
             atomicFactory.Dispose();
 
             holder.disposed.Should().BeTrue();
@@ -152,11 +160,10 @@ public async Task WhenCallersRunConcurrentlyResultIsFromWinner()
 
             result1.l.Value.actualNumber.Should().Be(winningNumber);
             result2.l.Value.actualNumber.Should().Be(winningNumber);
-                
+
             winnerCount.Should().Be(1);
         }
 
-
         [Fact]
         public async Task WhenCallersRunConcurrentlyWithFailureSameExceptionIsPropagated()
         {
@@ -199,6 +206,49 @@ await Task.WhenAll(first, second)
             }
         }
 
+        [Fact]
+        public async Task WhenCreateFromFactoryLifetimeThrowsDoesNotCauseUnobservedTaskException()
+        {
+            bool unobservedExceptionThrown = false;
+            TaskScheduler.UnobservedTaskException += OnUnobservedTaskException;
+
+            try
+            {
+                await ScopedAsyncAtomicFactoryTryCreateLifetimeAsync();
+
+                GC.Collect();
+                GC.WaitForPendingFinalizers();
+            }
+            finally
+            {
+                TaskScheduler.UnobservedTaskException -= OnUnobservedTaskException;
+            }
+
+            unobservedExceptionThrown.Should().BeFalse();
+
+            void OnUnobservedTaskException(object sender, UnobservedTaskExceptionEventArgs e)
+            {
+                outputHelper.WriteLine($"Unobserved task exception {e.Exception}");
+                unobservedExceptionThrown = true;
+                e.SetObserved();
+            }
+
+            static async Task ScopedAsyncAtomicFactoryTryCreateLifetimeAsync()
+            {
+                var a = new ScopedAsyncAtomicFactory<int, IntHolder>();
+                try
+                {
+                    _ = await a.TryCreateLifetimeAsync(1, k =>
+                    {
+                        throw new ArithmeticException();
+                    });
+                }
+                catch (ArithmeticException)
+                {
+                }
+            }
+        }
+
         [Fact]
         public async Task WhenDisposedWhileInitResultIsDisposed()
         {

BitFaster.Caching/Atomic/AsyncAtomicFactory.cs

@@ -159,7 +159,10 @@ public async ValueTask<V> CreateValueAsync<TFactory>(K key, TFactory valueFactor
                     {
                         Volatile.Write(ref isInitialized, false);
                         tcs.SetException(ex);
-                        throw;
+
+                        // always await the task to avoid unobserved task exceptions - normal case is that no other thread is waiting.
+                        // this will re-throw the exception.
+                        await tcs.Task.ConfigureAwait(false);
                     }
                 }
 

BitFaster.Caching/Atomic/ScopedAsyncAtomicFactory.cs

@@ -178,7 +178,10 @@ public async ValueTask<Scoped<V>> CreateScopeAsync<TFactory>(K key, TFactory val
                     {
                         Volatile.Write(ref isTaskInitialized, false);
                         tcs.SetException(ex);
-                        throw;
+
+                        // always await the task to avoid unobserved task exceptions - normal case is that no other thread is waiting.
+                        // this will re-throw the exception.
+                        await tcs.Task.ConfigureAwait(false);
                     }
                 }