From b718219336231ba8a78b07e8d18f8e82cfe2684c Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Sat, 29 Aug 2026 17:21:30 +1000 Subject: [PATCH 1/2] Await every registered combination callback UseCallbacks combined the callbacks with +=. Invoking a multicast Task returning delegate returns only the last target's Task, so when UseCallbacks was called more than once, for example by an extension package plus user code, every earlier callback ran unawaited: its exceptions unobserved and its async work racing the combination method. Extensions.Then documents the same hazard for Func. They are now held in lists and awaited in order. The no callback path still returns Task.CompletedTask without allocating. --- ...gisteredCallbackIsAwaited_run.verified.txt | 4 ++ src/StaticSettingsTests/CombinationTests.cs | 37 +++++++++++++ .../Combinations/CombinationSettings.cs | 53 +++++++++++++++---- 3 files changed, 83 insertions(+), 11 deletions(-) create mode 100644 src/StaticSettingsTests/CombinationTests.EveryRegisteredCallbackIsAwaited_run.verified.txt diff --git a/src/StaticSettingsTests/CombinationTests.EveryRegisteredCallbackIsAwaited_run.verified.txt b/src/StaticSettingsTests/CombinationTests.EveryRegisteredCallbackIsAwaited_run.verified.txt new file mode 100644 index 0000000000..f70460ad22 --- /dev/null +++ b/src/StaticSettingsTests/CombinationTests.EveryRegisteredCallbackIsAwaited_run.verified.txt @@ -0,0 +1,4 @@ +{ + list: Result, + 1: Exception: from the first before callback +} \ No newline at end of file diff --git a/src/StaticSettingsTests/CombinationTests.cs b/src/StaticSettingsTests/CombinationTests.cs index 71eea270f7..e5a67405d2 100644 --- a/src/StaticSettingsTests/CombinationTests.cs +++ b/src/StaticSettingsTests/CombinationTests.cs @@ -79,6 +79,43 @@ await Verify(new .UseMethodName("CallbackResults"); } + // Two registrations, for example an extension package plus user code. A multicast + // delegate would return only the last target's Task, leaving the first to run + // unawaited: its exceptions unobserved and its work racing the combination method. + [Fact] + public async Task EveryRegisteredCallbackIsAwaited() + { + var exceptionMessages = new List(); + + // Fails only after an await, so the failure is carried by the returned Task rather + // than thrown synchronously. Registered first, so a multicast delegate would return + // the second registration's Task instead and this one would never be observed. + CombinationSettings.UseCallbacks( + async _ => + { + await Task.Yield(); + throw new("from the first before callback"); + }, + (_, _) => Task.CompletedTask, + (_, exception) => + { + exceptionMessages.Add(exception.Message); + return Task.CompletedTask; + }); + + CombinationSettings.UseCallbacks( + _ => Task.CompletedTask, + (_, _) => Task.CompletedTask, + (_, _) => Task.CompletedTask); + + int[] list = [1]; + await Combination() + .Verify((int param1) => param1, list) + .UseMethodName("EveryRegisteredCallbackIsAwaited_run"); + + Assert.Equal(["from the first before callback"], exceptionMessages); + } + [Fact] public async Task AfterCallbackRawValueWhenRecording() { diff --git a/src/Verify/Combinations/CombinationSettings.cs b/src/Verify/Combinations/CombinationSettings.cs index 0ef3ab9d19..2f03721075 100644 --- a/src/Verify/Combinations/CombinationSettings.cs +++ b/src/Verify/Combinations/CombinationSettings.cs @@ -1,4 +1,4 @@ -namespace VerifyTests; +namespace VerifyTests; public delegate Task BeforeCombination(IReadOnlyList keys); public delegate Task AfterCombination(IReadOnlyList keys, object? result); @@ -16,7 +16,11 @@ public static void IncludeHeaders() => public static void CaptureExceptions() => CaptureExceptionsEnabled = true; - static BeforeCombination? before; + // Held as lists rather than combined with `+=`. Invoking a multicast Task returning + // delegate returns only the last target's Task, so every earlier callback would run + // unawaited: its exceptions unobserved, and its async work racing the combination + // method. Same hazard the Then extension documents for Func. + static List? before; internal static Task RunBeforeCallbacks(IReadOnlyList keys) { @@ -25,10 +29,18 @@ internal static Task RunBeforeCallbacks(IReadOnlyList keys) return Task.CompletedTask; } - return before(keys); + return RunAll(before, keys); } - static AfterCombination? after; + static async Task RunAll(List callbacks, IReadOnlyList keys) + { + foreach (var callback in callbacks) + { + await callback(keys); + } + } + + static List? after; internal static Task RunAfterCallbacks(IReadOnlyList keys, object? result) { @@ -37,10 +49,18 @@ internal static Task RunAfterCallbacks(IReadOnlyList keys, object? resu return Task.CompletedTask; } - return after(keys, result); + return RunAll(after, keys, result); } - static CombinationException? combinationException; + static async Task RunAll(List callbacks, IReadOnlyList keys, object? result) + { + foreach (var callback in callbacks) + { + await callback(keys, result); + } + } + + static List? combinationException; internal static Task RunExceptionCallbacks(IReadOnlyList keys, Exception exception) { @@ -49,14 +69,25 @@ internal static Task RunExceptionCallbacks(IReadOnlyList keys, Exceptio return Task.CompletedTask; } - return combinationException(keys, exception); + return RunAll(combinationException, keys, exception); + } + + static async Task RunAll(List callbacks, IReadOnlyList keys, Exception exception) + { + foreach (var callback in callbacks) + { + await callback(keys, exception); + } } public static void UseCallbacks(BeforeCombination before, AfterCombination after, CombinationException exception) { - CombinationSettings.before += before; - CombinationSettings.after += after; - combinationException += exception; + CombinationSettings.before ??= []; + CombinationSettings.before.Add(before); + CombinationSettings.after ??= []; + CombinationSettings.after.Add(after); + combinationException ??= []; + combinationException.Add(exception); } public static void Reset() @@ -67,4 +98,4 @@ public static void Reset() after = null; before = null; } -} \ No newline at end of file +} From e32d0e7417b314d6f3a8774ecf67e6f639da3b78 Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Sat, 29 Aug 2026 07:32:46 +0000 Subject: [PATCH 2/2] Docs changes --- docs/combinations.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/combinations.md b/docs/combinations.md index a524e03aa3..fc5a98d293 100644 --- a/docs/combinations.md +++ b/docs/combinations.md @@ -175,7 +175,7 @@ public Task BuildAddressExceptionsDisabledTest() city); } ``` -snippet source | anchor +snippet source | anchor @@ -401,7 +401,7 @@ class CustomCombinationConverter : string.Join(", ", keys.Select(_ => _.Value)); } ``` -snippet source | anchor +snippet source | anchor Full control of serialization can be achieved by inheriting from `WriteOnlyJsonConverter`. @@ -420,7 +420,7 @@ static CustomCombinationConverter customConverter = new(); public static void Init() => VerifierSettings.AddExtraSettings(_ => _.Converters.Insert(0, customConverter)); ``` -snippet source | anchor +snippet source | anchor @@ -551,5 +551,5 @@ Headers can be enabled globally: public static void EnableIncludeHeaders() => CombinationSettings.IncludeHeaders(); ``` -snippet source | anchor +snippet source | anchor