From 7980b2367c183f2fb342e07a4ece4d5ec1284732 Mon Sep 17 00:00:00 2001 From: jonas Date: Mon, 9 Jun 2025 11:57:07 +0200 Subject: [PATCH 1/7] AsyncAtomicFactory.Initializer: do not call SetException on TCS to avoid unobserved task exception when garbage collecting TCS --- .../Atomic/AsyncAtomicFactoryTests.cs | 38 +++++++++++++++++++ .../Atomic/AsyncAtomicFactory.cs | 3 +- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs b/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs index fdb8a6f8..5ba8b054 100644 --- a/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs +++ b/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs @@ -75,6 +75,44 @@ public async Task WhenValueCreateThrowsValueIsNotStored() (await a.GetValueAsync(1, k => Task.FromResult(3))).Should().Be(3); } + [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) + { + unobservedExceptionThrown = true; + e.SetObserved(); + } + + static async Task AsyncAtomicFactoryGetValueAsync() + { + var a = new AsyncAtomicFactory(); + try + { + _ = await a.GetValueAsync(12, (i) => throw new ArithmeticException()); + } + catch (ArithmeticException) + { + } + } + } + [Fact] public async Task WhenCallersRunConcurrentlyResultIsFromWinner() { diff --git a/BitFaster.Caching/Atomic/AsyncAtomicFactory.cs b/BitFaster.Caching/Atomic/AsyncAtomicFactory.cs index 0588d0a9..e65c4df6 100644 --- a/BitFaster.Caching/Atomic/AsyncAtomicFactory.cs +++ b/BitFaster.Caching/Atomic/AsyncAtomicFactory.cs @@ -155,10 +155,9 @@ public async ValueTask CreateValueAsync(K key, TFactory valueFactor return value; } - catch (Exception ex) + catch (Exception) { Volatile.Write(ref isInitialized, false); - tcs.SetException(ex); throw; } } From b0c48d65eb7b77b5fe46ef44e50feaa25e249fad Mon Sep 17 00:00:00 2001 From: jonas Date: Wed, 11 Jun 2025 17:54:34 +0200 Subject: [PATCH 2/7] Consider other exception type on build server --- .../Atomic/AsyncAtomicFactoryTests.cs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs b/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs index 5ba8b054..3d431c77 100644 --- a/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs +++ b/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs @@ -96,7 +96,15 @@ public async Task WhenValueCreateThrowsDoesNotCauseUnobservedTaskException() void OnUnobservedTaskException(object sender, UnobservedTaskExceptionEventArgs e) { - unobservedExceptionThrown = true; + if (e.Exception?.InnerException is ArithmeticException) + { + unobservedExceptionThrown = true; + } + else + { + Assert.Fail(e.Exception?.ToString()); + } + e.SetObserved(); } @@ -110,6 +118,8 @@ static async Task AsyncAtomicFactoryGetValueAsync() catch (ArithmeticException) { } + + await a.GetValueAsync(12, (i) => Task.FromResult(24)); } } From d36a511bc3b4c91e1456d22bfd796d7fe651866d Mon Sep 17 00:00:00 2001 From: jonas Date: Wed, 11 Jun 2025 17:57:17 +0200 Subject: [PATCH 3/7] Remove second GetValueAsync call --- BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs b/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs index 3d431c77..06cf0b99 100644 --- a/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs +++ b/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs @@ -118,8 +118,6 @@ static async Task AsyncAtomicFactoryGetValueAsync() catch (ArithmeticException) { } - - await a.GetValueAsync(12, (i) => Task.FromResult(24)); } } From aa89389683ea61188ce7eda9b05a2548fb127110 Mon Sep 17 00:00:00 2001 From: jonas Date: Thu, 12 Jun 2025 22:15:27 +0200 Subject: [PATCH 4/7] ScopedAsyncAtomicFactory.Initializer: avoid unobserved task exception as well Remove test in AsyncAtomicFactoryTests again --- .../Atomic/AsyncAtomicFactoryTests.cs | 46 ------------------- .../Atomic/ScopedAsyncAtomicFactory.cs | 3 +- 2 files changed, 1 insertion(+), 48 deletions(-) diff --git a/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs b/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs index 06cf0b99..fdb8a6f8 100644 --- a/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs +++ b/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs @@ -75,52 +75,6 @@ public async Task WhenValueCreateThrowsValueIsNotStored() (await a.GetValueAsync(1, k => Task.FromResult(3))).Should().Be(3); } - [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) - { - if (e.Exception?.InnerException is ArithmeticException) - { - unobservedExceptionThrown = true; - } - else - { - Assert.Fail(e.Exception?.ToString()); - } - - e.SetObserved(); - } - - static async Task AsyncAtomicFactoryGetValueAsync() - { - var a = new AsyncAtomicFactory(); - try - { - _ = await a.GetValueAsync(12, (i) => throw new ArithmeticException()); - } - catch (ArithmeticException) - { - } - } - } - [Fact] public async Task WhenCallersRunConcurrentlyResultIsFromWinner() { diff --git a/BitFaster.Caching/Atomic/ScopedAsyncAtomicFactory.cs b/BitFaster.Caching/Atomic/ScopedAsyncAtomicFactory.cs index 594a609b..90fae43d 100644 --- a/BitFaster.Caching/Atomic/ScopedAsyncAtomicFactory.cs +++ b/BitFaster.Caching/Atomic/ScopedAsyncAtomicFactory.cs @@ -174,10 +174,9 @@ public async ValueTask> CreateScopeAsync(K key, TFactory val return scope; } - catch (Exception ex) + catch (Exception) { Volatile.Write(ref isTaskInitialized, false); - tcs.SetException(ex); throw; } } From 843e8b97de34a79d3492823fa647db9e4834dfd5 Mon Sep 17 00:00:00 2001 From: Alex Peck Date: Fri, 13 Jun 2025 15:42:00 -0700 Subject: [PATCH 5/7] fix --- .../Atomic/AsyncAtomicFactoryTests.cs | 39 +++++++++++++++++ .../Atomic/ScopedAsyncAtomicFactoryTests.cs | 43 ++++++++++++++++++- .../Atomic/AsyncAtomicFactory.cs | 5 ++- .../Atomic/ScopedAsyncAtomicFactory.cs | 5 ++- 4 files changed, 89 insertions(+), 3 deletions(-) diff --git a/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs b/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs index b47f414e..44ffef70 100644 --- a/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs +++ b/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs @@ -156,6 +156,45 @@ 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) + { + unobservedExceptionThrown = true; + e.SetObserved(); + } + + static async Task AsyncAtomicFactoryGetValueAsync() + { + var a = new AsyncAtomicFactory(); + try + { + _ = await a.GetValueAsync(12, i => throw new ArithmeticException()); + } + catch (ArithmeticException) + { + } + } + } + [Fact] public void WhenValueNotCreatedHashCodeIsZero() { diff --git a/BitFaster.Caching.UnitTests/Atomic/ScopedAsyncAtomicFactoryTests.cs b/BitFaster.Caching.UnitTests/Atomic/ScopedAsyncAtomicFactoryTests.cs index c2705bc7..cc6f7244 100644 --- a/BitFaster.Caching.UnitTests/Atomic/ScopedAsyncAtomicFactoryTests.cs +++ b/BitFaster.Caching.UnitTests/Atomic/ScopedAsyncAtomicFactoryTests.cs @@ -156,7 +156,6 @@ public async Task WhenCallersRunConcurrentlyResultIsFromWinner() winnerCount.Should().Be(1); } - [Fact] public async Task WhenCallersRunConcurrentlyWithFailureSameExceptionIsPropagated() { @@ -199,6 +198,48 @@ 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) + { + unobservedExceptionThrown = true; + e.SetObserved(); + } + + static async Task AsyncAtomicFactoryGetValueAsync() + { + var a = new ScopedAsyncAtomicFactory(); + try + { + _ = await a.TryCreateLifetimeAsync(1, k => + { + throw new ArithmeticException(); + }); + } + catch (ArithmeticException) + { + } + } + } + [Fact] public async Task WhenDisposedWhileInitResultIsDisposed() { diff --git a/BitFaster.Caching/Atomic/AsyncAtomicFactory.cs b/BitFaster.Caching/Atomic/AsyncAtomicFactory.cs index 0588d0a9..445a6ba9 100644 --- a/BitFaster.Caching/Atomic/AsyncAtomicFactory.cs +++ b/BitFaster.Caching/Atomic/AsyncAtomicFactory.cs @@ -159,7 +159,10 @@ public async ValueTask CreateValueAsync(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 task is waiting. + // this will re-throw the exception. + await tcs.Task.ConfigureAwait(false); } } diff --git a/BitFaster.Caching/Atomic/ScopedAsyncAtomicFactory.cs b/BitFaster.Caching/Atomic/ScopedAsyncAtomicFactory.cs index 594a609b..4431fc6a 100644 --- a/BitFaster.Caching/Atomic/ScopedAsyncAtomicFactory.cs +++ b/BitFaster.Caching/Atomic/ScopedAsyncAtomicFactory.cs @@ -178,7 +178,10 @@ public async ValueTask> CreateScopeAsync(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 task is waiting. + // this will re-throw the exception. + await tcs.Task.ConfigureAwait(false); } } From 828ed649a09c13c2eda487b49637f2583d17bcfa Mon Sep 17 00:00:00 2001 From: Alex Peck Date: Fri, 13 Jun 2025 15:57:08 -0700 Subject: [PATCH 6/7] task => thread --- BitFaster.Caching/Atomic/AsyncAtomicFactory.cs | 2 +- BitFaster.Caching/Atomic/ScopedAsyncAtomicFactory.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/BitFaster.Caching/Atomic/AsyncAtomicFactory.cs b/BitFaster.Caching/Atomic/AsyncAtomicFactory.cs index 445a6ba9..ff48ce40 100644 --- a/BitFaster.Caching/Atomic/AsyncAtomicFactory.cs +++ b/BitFaster.Caching/Atomic/AsyncAtomicFactory.cs @@ -160,7 +160,7 @@ public async ValueTask CreateValueAsync(K key, TFactory valueFactor Volatile.Write(ref isInitialized, false); tcs.SetException(ex); - // always await the task to avoid unobserved task exceptions - normal case is that no other task is waiting. + // 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); } diff --git a/BitFaster.Caching/Atomic/ScopedAsyncAtomicFactory.cs b/BitFaster.Caching/Atomic/ScopedAsyncAtomicFactory.cs index 4431fc6a..839b51a1 100644 --- a/BitFaster.Caching/Atomic/ScopedAsyncAtomicFactory.cs +++ b/BitFaster.Caching/Atomic/ScopedAsyncAtomicFactory.cs @@ -179,7 +179,7 @@ public async ValueTask> CreateScopeAsync(K key, TFactory val Volatile.Write(ref isTaskInitialized, false); tcs.SetException(ex); - // always await the task to avoid unobserved task exceptions - normal case is that no other task is waiting. + // 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); } From 41e7272ea770fb509217f4dec102dc535f480f9c Mon Sep 17 00:00:00 2001 From: jonas Date: Sun, 15 Jun 2025 15:07:38 +0200 Subject: [PATCH 7/7] Rename test in ScopedAsyncAtomicFactoryTests and log unobserved task exception to test output. --- .../Atomic/AsyncAtomicFactoryTests.cs | 9 +++++++++ .../Atomic/ScopedAsyncAtomicFactoryTests.cs | 19 ++++++++++++++----- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs b/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs index 44ffef70..7723cc43 100644 --- a/BitFaster.Caching.UnitTests/Atomic/AsyncAtomicFactoryTests.cs +++ b/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() { @@ -178,6 +186,7 @@ public async Task WhenValueCreateThrowsDoesNotCauseUnobservedTaskException() void OnUnobservedTaskException(object sender, UnobservedTaskExceptionEventArgs e) { + outputHelper.WriteLine($"Unobserved task exception {e.Exception}"); unobservedExceptionThrown = true; e.SetObserved(); } diff --git a/BitFaster.Caching.UnitTests/Atomic/ScopedAsyncAtomicFactoryTests.cs b/BitFaster.Caching.UnitTests/Atomic/ScopedAsyncAtomicFactoryTests.cs index cc6f7244..7805b5d4 100644 --- a/BitFaster.Caching.UnitTests/Atomic/ScopedAsyncAtomicFactoryTests.cs +++ b/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(holder); - + atomicFactory.Dispose(); holder.disposed.Should().BeTrue(); @@ -152,7 +160,7 @@ public async Task WhenCallersRunConcurrentlyResultIsFromWinner() result1.l.Value.actualNumber.Should().Be(winningNumber); result2.l.Value.actualNumber.Should().Be(winningNumber); - + winnerCount.Should().Be(1); } @@ -199,14 +207,14 @@ await Task.WhenAll(first, second) } [Fact] - public async Task WhenValueCreateThrowsDoesNotCauseUnobservedTaskException() + public async Task WhenCreateFromFactoryLifetimeThrowsDoesNotCauseUnobservedTaskException() { bool unobservedExceptionThrown = false; TaskScheduler.UnobservedTaskException += OnUnobservedTaskException; try { - await AsyncAtomicFactoryGetValueAsync(); + await ScopedAsyncAtomicFactoryTryCreateLifetimeAsync(); GC.Collect(); GC.WaitForPendingFinalizers(); @@ -220,11 +228,12 @@ public async Task WhenValueCreateThrowsDoesNotCauseUnobservedTaskException() void OnUnobservedTaskException(object sender, UnobservedTaskExceptionEventArgs e) { + outputHelper.WriteLine($"Unobserved task exception {e.Exception}"); unobservedExceptionThrown = true; e.SetObserved(); } - static async Task AsyncAtomicFactoryGetValueAsync() + static async Task ScopedAsyncAtomicFactoryTryCreateLifetimeAsync() { var a = new ScopedAsyncAtomicFactory(); try