Skip to content

Commit 9c4c40f

Browse files
authored
fix: resolve pre-4.0 audit findings across streaming and cancellation paths (#370)
* fix: resolve pre-4.0 audit findings across streaming and cancellation paths (#362-#369) Exception fidelity (#362): the Task returned by the IAsyncEnumerable processors' ExecuteAsync is now built from a TaskCompletionSource, so Task.Exception.InnerExceptions carries every failure (Task.WhenAll fidelity) instead of discarding all but the first. Applied to the worker pool, parallel, batch (whole failing batch preserved), and one-at-a-time (multi-fault item task preserved) processors via a new internal StreamingExecution helper. Abandonment semantics (#363, #364): breaking out of a result stream now cancels the processor's in-flight work, drains it within the 30s disposal window, and observes failures so they neither mask the propagating exception nor surface as UnobservedTaskException. The void twin no longer masks a mid-enumeration source failure with an in-flight task failure. The unbounded ProcessInParallel extension drains started tasks on mid-enumeration failure instead of abandoning them fire-and-forget. Ordering contract (#365): documented per method - bounded streaming and the awaitable extension yield source order, unbounded streaming and GetResultsAsyncEnumerable yield completion order - in XML docs and a new README section. ConfigureAwait gaps (#366): added ConfigureAwait(false) to every WithCancellation enumeration in the SelectMany/SelectManyAsync iterator families. Pre-cancelled token (#367): building a processor with an already-cancelled token now yields cancelled per-item tasks (TPL convention) instead of ArgumentException naming an internal constructor parameter. The registration in ProcessorLifecycle.Start fires synchronously on the fully built instance. CancelAll/Dispose race (#368): CancelAllCore tolerates a concurrent Dispose winning the race to the CancellationTokenSource, matching the streaming processors' existing handling. Single-use enforcement (#369): a second ExecuteAsync call throws InvalidOperationException eagerly at the call site; a call after disposal throws ObjectDisposedException naming the processor type. Fixes #362, #363, #364, #365, #366, #367, #368, #369. * fix: address review - guard disposal snapshot race, clarify batch fault collection The single-use guard now re-reads the disposed flag after claiming execution (the Interlocked.Exchange is a full fence), so a disposal that completed before the ExecuteAsync call is always seen instead of a stale pre-claim snapshot. ProcessBatch's fault collection is inlined without the dead wasCanceled local: the catch filter already routes pure cancellation to the caller's OperationCanceledException handler, which completes the execution task as canceled. A regression test pins that contract.
1 parent 53ae204 commit 9c4c40f

20 files changed

Lines changed: 978 additions & 178 deletions
Lines changed: 265 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,265 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Diagnostics;
4+
using System.Linq;
5+
using System.Threading;
6+
using System.Threading.Tasks;
7+
using EnumerableAsyncProcessor.Extensions;
8+
using TUnit.Assertions;
9+
using TUnit.Core;
10+
11+
namespace EnumerableAsyncProcessor.UnitTests;
12+
13+
/// <summary>
14+
/// Guards exception fidelity and abandonment semantics on the IAsyncEnumerable paths:
15+
/// - the Task from void ExecuteAsync must carry every failure via Task.Exception.InnerExceptions,
16+
/// matching the IEnumerable processors (issue #362),
17+
/// - a mid-enumeration source failure must stay the primary exception instead of being masked
18+
/// by an in-flight task failure (issue #363),
19+
/// - breaking out of an unbounded result stream must cancel in-flight work instead of silently
20+
/// blocking until it finishes naturally (issue #363),
21+
/// - the unbounded ProcessInParallel extension must drain started tasks on mid-enumeration
22+
/// failure instead of abandoning them fire-and-forget (issue #364).
23+
/// </summary>
24+
public class AsyncEnumerableExceptionFidelityTests
25+
{
26+
private static async IAsyncEnumerable<int> Source(int count)
27+
{
28+
for (var i = 0; i < count; i++)
29+
{
30+
await Task.Yield();
31+
yield return i;
32+
}
33+
}
34+
35+
private static async IAsyncEnumerable<int> ThrowingSource(int yieldBeforeThrow)
36+
{
37+
for (var i = 0; i < yieldBeforeThrow; i++)
38+
{
39+
await Task.Yield();
40+
yield return i;
41+
}
42+
43+
throw new InvalidOperationException("source failed");
44+
}
45+
46+
[Test]
47+
public async Task Bounded_Void_ExecuteAsync_Task_Carries_Every_Failure()
48+
{
49+
var executeTask = Source(10)
50+
.ForEachAsync(i => (i is 2 or 5 or 8)
51+
? Task.FromException(new InvalidOperationException($"item {i} failed"))
52+
: Task.CompletedTask)
53+
.ProcessInParallel(maxConcurrency: 2)
54+
.ExecuteAsync();
55+
56+
Exception? caught = null;
57+
try
58+
{
59+
await executeTask;
60+
}
61+
catch (Exception exception)
62+
{
63+
caught = exception;
64+
}
65+
66+
await Assert.That(caught is InvalidOperationException).IsTrue();
67+
68+
var messages = executeTask.Exception!.InnerExceptions.Select(e => e.Message).OrderBy(m => m).ToList();
69+
await Assert.That(messages.Count).IsEqualTo(3);
70+
await Assert.That(messages).IsEquivalentTo(new[] { "item 2 failed", "item 5 failed", "item 8 failed" });
71+
}
72+
73+
[Test]
74+
public async Task Batch_Void_ExecuteAsync_Task_Carries_Every_Failure_In_The_Batch()
75+
{
76+
var executeTask = Source(2)
77+
.ForEachAsync(i => Task.FromException(new InvalidOperationException($"item {i} failed")))
78+
.ProcessInBatches(2)
79+
.ExecuteAsync();
80+
81+
try
82+
{
83+
await executeTask;
84+
}
85+
catch (Exception)
86+
{
87+
// Expected
88+
}
89+
90+
var messages = executeTask.Exception!.InnerExceptions.Select(e => e.Message).ToList();
91+
await Assert.That(messages.Count).IsEqualTo(2);
92+
await Assert.That(messages).IsEquivalentTo(new[] { "item 0 failed", "item 1 failed" });
93+
}
94+
95+
[Test]
96+
public async Task Batch_Void_All_Items_Cancelled_Completes_As_Cancelled_Not_Successful()
97+
{
98+
using var itemCancellation = new CancellationTokenSource();
99+
itemCancellation.Cancel();
100+
101+
var executeTask = Source(2)
102+
.ForEachAsync(_ => Task.FromCanceled(itemCancellation.Token))
103+
.ProcessInBatches(2)
104+
.ExecuteAsync();
105+
106+
Exception? caught = null;
107+
try
108+
{
109+
await executeTask;
110+
}
111+
catch (Exception exception)
112+
{
113+
caught = exception;
114+
}
115+
116+
await Assert.That(caught is OperationCanceledException).IsTrue();
117+
await Assert.That(executeTask.IsCanceled).IsTrue();
118+
}
119+
120+
[Test]
121+
public async Task Unbounded_Void_Source_Failure_Stays_Primary_When_InFlight_Tasks_Also_Fail()
122+
{
123+
var taskStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
124+
125+
var executeTask = ThrowingSource(1)
126+
.ForEachAsync(async _ =>
127+
{
128+
taskStarted.TrySetResult();
129+
await Task.Yield();
130+
throw new ApplicationException("in-flight task failed");
131+
})
132+
.ProcessInParallel()
133+
.ExecuteAsync();
134+
135+
await taskStarted.Task;
136+
137+
Exception? caught = null;
138+
try
139+
{
140+
await executeTask;
141+
}
142+
catch (Exception exception)
143+
{
144+
caught = exception;
145+
}
146+
147+
// The source failure is primary; the in-flight failure is preserved alongside it
148+
await Assert.That(caught is InvalidOperationException).IsTrue();
149+
await Assert.That(caught!.Message).IsEqualTo("source failed");
150+
151+
var messages = executeTask.Exception!.InnerExceptions.Select(e => e.Message).ToList();
152+
await Assert.That(messages.Count).IsEqualTo(2);
153+
await Assert.That(messages.Contains("in-flight task failed")).IsTrue();
154+
}
155+
156+
[Test]
157+
public async Task Unbounded_Result_Stream_Early_Break_Cancels_InFlight_Work()
158+
{
159+
var cancelledCount = 0;
160+
161+
var processor = Source(10)
162+
.SelectAsync(async (i, cancellationToken) =>
163+
{
164+
if (i == 0)
165+
{
166+
return i;
167+
}
168+
169+
try
170+
{
171+
await Task.Delay(Timeout.Infinite, cancellationToken);
172+
}
173+
catch (OperationCanceledException)
174+
{
175+
Interlocked.Increment(ref cancelledCount);
176+
throw;
177+
}
178+
179+
return i;
180+
})
181+
.ProcessInParallel();
182+
183+
var stopwatch = Stopwatch.StartNew();
184+
185+
await foreach (var _ in processor.ExecuteAsync())
186+
{
187+
break;
188+
}
189+
190+
stopwatch.Stop();
191+
192+
// Before the fix this blocked forever (the drain waited for every in-flight task and
193+
// nothing cancelled them). Well under the 30s disposal window proves cancellation ran.
194+
await Assert.That(stopwatch.Elapsed < TimeSpan.FromSeconds(15)).IsTrue();
195+
await Assert.That(cancelledCount).IsEqualTo(9);
196+
}
197+
198+
[Test]
199+
public async Task Bounded_Result_Stream_Early_Break_Cancels_InFlight_Work()
200+
{
201+
var cancelledCount = 0;
202+
203+
var processor = Source(10)
204+
.SelectAsync(async (i, cancellationToken) =>
205+
{
206+
if (i == 0)
207+
{
208+
return i;
209+
}
210+
211+
try
212+
{
213+
await Task.Delay(Timeout.Infinite, cancellationToken);
214+
}
215+
catch (OperationCanceledException)
216+
{
217+
Interlocked.Increment(ref cancelledCount);
218+
throw;
219+
}
220+
221+
return i;
222+
})
223+
.ProcessInParallel(maxConcurrency: 3);
224+
225+
var stopwatch = Stopwatch.StartNew();
226+
227+
await foreach (var _ in processor.ExecuteAsync())
228+
{
229+
break;
230+
}
231+
232+
stopwatch.Stop();
233+
234+
await Assert.That(stopwatch.Elapsed < TimeSpan.FromSeconds(15)).IsTrue();
235+
await Assert.That(cancelledCount > 0).IsTrue();
236+
}
237+
238+
[Test]
239+
public async Task Unbounded_Extension_Drains_Started_Tasks_On_MidEnumeration_Failure()
240+
{
241+
var completedCount = 0;
242+
243+
Exception? caught = null;
244+
try
245+
{
246+
_ = await ThrowingSource(3).ProcessInParallel(async i =>
247+
{
248+
await Task.Delay(100);
249+
Interlocked.Increment(ref completedCount);
250+
return i;
251+
});
252+
}
253+
catch (Exception exception)
254+
{
255+
caught = exception;
256+
}
257+
258+
await Assert.That(caught is InvalidOperationException).IsTrue();
259+
await Assert.That(caught!.Message).IsEqualTo("source failed");
260+
261+
// Before the fix the extension rethrew immediately and abandoned the started tasks
262+
// fire-and-forget; now they are drained before the exception leaves the method.
263+
await Assert.That(completedCount).IsEqualTo(3);
264+
}
265+
}
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
using System;
2+
using System.Linq;
3+
using System.Threading;
4+
using System.Threading.Tasks;
5+
using EnumerableAsyncProcessor.Extensions;
6+
using TUnit.Assertions;
7+
using TUnit.Core;
8+
9+
namespace EnumerableAsyncProcessor.UnitTests;
10+
11+
/// <summary>
12+
/// Guards the pre-cancelled token contract (issue #367):
13+
/// building a processor with an already-cancelled token must produce a processor whose
14+
/// per-item tasks are cancelled - matching TPL convention - instead of throwing
15+
/// ArgumentException from an internal constructor parameter the caller never passed.
16+
/// </summary>
17+
public class PreCancelledTokenTests
18+
{
19+
[Test]
20+
public async Task Void_Processor_Built_With_PreCancelled_Token_Yields_Cancelled_Tasks()
21+
{
22+
using var cancellationTokenSource = new CancellationTokenSource();
23+
cancellationTokenSource.Cancel();
24+
25+
var invoked = 0;
26+
27+
await using var processor = Enumerable.Range(0, 5).ToList()
28+
.ForEachAsync(_ =>
29+
{
30+
Interlocked.Increment(ref invoked);
31+
return Task.CompletedTask;
32+
}, cancellationTokenSource.Token)
33+
.ProcessInParallel(maxConcurrency: 2);
34+
35+
Exception? caught = null;
36+
try
37+
{
38+
await processor.WaitAsync();
39+
}
40+
catch (Exception exception)
41+
{
42+
caught = exception;
43+
}
44+
45+
await Assert.That(caught is OperationCanceledException).IsTrue();
46+
await Assert.That(processor.GetEnumerableTasks().All(t => t.IsCanceled)).IsTrue();
47+
await Assert.That(invoked).IsEqualTo(0);
48+
}
49+
50+
[Test]
51+
public async Task Result_Processor_Built_With_PreCancelled_Token_Yields_Cancelled_Tasks()
52+
{
53+
using var cancellationTokenSource = new CancellationTokenSource();
54+
cancellationTokenSource.Cancel();
55+
56+
await using var processor = Enumerable.Range(0, 5).ToList()
57+
.SelectAsync(i => Task.FromResult(i), cancellationTokenSource.Token)
58+
.ProcessInParallel(maxConcurrency: 2);
59+
60+
Exception? caught = null;
61+
try
62+
{
63+
_ = await processor.GetResultsAsync();
64+
}
65+
catch (Exception exception)
66+
{
67+
caught = exception;
68+
}
69+
70+
await Assert.That(caught is OperationCanceledException).IsTrue();
71+
await Assert.That(processor.GetEnumerableTasks().All(t => t.IsCanceled)).IsTrue();
72+
}
73+
74+
[Test]
75+
public async Task OneAtATime_And_Batch_Built_With_PreCancelled_Token_Yield_Cancelled_Tasks()
76+
{
77+
using var cancellationTokenSource = new CancellationTokenSource();
78+
cancellationTokenSource.Cancel();
79+
80+
await using var oneAtATime = new[] { 1, 2, 3 }
81+
.ForEachAsync(_ => Task.CompletedTask, cancellationTokenSource.Token)
82+
.ProcessOneAtATime();
83+
84+
await using var batched = new[] { 1, 2, 3 }
85+
.ForEachAsync(_ => Task.CompletedTask, cancellationTokenSource.Token)
86+
.ProcessInBatches(2);
87+
88+
await Assert.That(oneAtATime.GetEnumerableTasks().All(t => t.IsCanceled)).IsTrue();
89+
await Assert.That(batched.GetEnumerableTasks().All(t => t.IsCanceled)).IsTrue();
90+
}
91+
}

0 commit comments

Comments
 (0)