test(crowdsec): pin the drain task to the loop's lifetime

Every other reporter test reaches Stop() without a Start(), so the drain task
was never exercised and the Task<ValueTask> defect passed the whole suite while
doing nothing. Race the drain against a delay: with an empty queue the loop
parks on WaitToReadAsync forever, so a correctly unwrapped task cannot win that
race, while the Task<ValueTask> completed in ~0ms. Then assert Stop() does not
return until the loop has actually exited, which is the precondition the flush
relies on before reading the SingleReader channel.

Verified by reverting DrainLoop to ValueTask: the test fails. A slow pool can
only make it under-detect, never fail spuriously.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Kamron Batman 2026-07-25 11:11:48 -07:00
parent 6f74532280
commit 0f64d834b4
No known key found for this signature in database
GPG key ID: 7D81DF26D9A5D94A
2 changed files with 44 additions and 0 deletions

View file

@ -160,6 +160,44 @@ public class CrowdSecReporterTests
Assert.Equal(0, reporter.SendFailureCount);
}
/// <summary>
/// Regression for the two shapes that made the drain task lie about the loop's lifetime.
/// </summary>
/// <remarks>
/// <see cref="CrowdSecReporter.Start"/> hands the drain loop to <see cref="Task.Run(Func{Task})"/>,
/// and there is no <c>Task.Run(Func&lt;ValueTask&gt;)</c> overload — a ValueTask-returning drain loop
/// binds to <c>Task.Run&lt;TResult&gt;(Func&lt;TResult&gt;)</c> and yields a <c>Task&lt;ValueTask&gt;</c>
/// that completes at the first suspending await rather than when the loop exits, silently upcast by
/// the <see cref="Task"/> field. That turned <see cref="CrowdSecReporter.Stop"/>'s drain-exited
/// handshake into a no-op and let the shutdown flush read the <c>SingleReader</c> channel while the
/// drain loop was still reading it. Every other test here reaches Stop() without a Start(), so
/// nothing covered it.
/// <para>
/// Racing the drain against a delay is what separates the two shapes: with an empty queue the loop
/// parks on <c>WaitToReadAsync</c> forever, so a correctly unwrapped task cannot win that race, while
/// the <c>Task&lt;ValueTask&gt;</c> completed in ~0ms. A slow pool can only make this under-detect,
/// never fail spuriously.
/// </para>
/// </remarks>
[Fact]
public async Task Start_DrainTaskSpansLoopLifetime_NotJustTheFirstAwait()
{
var reporter = new CrowdSecReporter(new NullAlertClient(), Settings());
using var cts = new CancellationTokenSource();
reporter.Start(cts.Token);
var drain = reporter.DrainTaskForTesting;
Assert.NotNull(drain);
var first = await Task.WhenAny(drain, Task.Delay(TimeSpan.FromMilliseconds(500)));
Assert.False(ReferenceEquals(first, drain), "drain task completed while the loop was still running");
reporter.Stop();
Assert.True(drain.IsCompleted, "Stop() returned before the drain loop exited");
}
private sealed class NullAlertClient : ICrowdSecAlertClient
{
public ValueTask PostAlertsAsync(IReadOnlyList<CrowdSecAlert> alerts, CancellationToken token) =>

View file

@ -69,6 +69,12 @@ public sealed class CrowdSecReporter : IBanReporter
/// </summary>
public int SendFailureCount => _sendFailures;
/// <summary>
/// The drain loop's task, so tests can observe its lifetime. It must stay incomplete until the loop
/// actually exits — see the note on <see cref="DrainLoop"/> for the shape that silently broke that.
/// </summary>
internal Task DrainTaskForTesting => _drainTask;
public static void Configure()
{
BanChannel.Register(new CrowdSecReporter());