fix(saf): publish _sweepTask only on CAS win so StopAsync drains the real in-flight sweep (plan R2-02 T8)
This commit is contained in:
@@ -394,7 +394,7 @@ public class StoreAndForwardService
|
|||||||
});
|
});
|
||||||
|
|
||||||
_retryTimer = new Timer(
|
_retryTimer = new Timer(
|
||||||
_ => Volatile.Write(ref _sweepTask, RetryPendingMessagesAsync()),
|
_ => KickSweep(),
|
||||||
null,
|
null,
|
||||||
_options.RetryTimerInterval,
|
_options.RetryTimerInterval,
|
||||||
_options.RetryTimerInterval);
|
_options.RetryTimerInterval);
|
||||||
@@ -658,17 +658,37 @@ public class StoreAndForwardService
|
|||||||
/// <summary>
|
/// <summary>
|
||||||
/// Kicks a background retry sweep now (fire-and-forget). No-op before
|
/// Kicks a background retry sweep now (fire-and-forget). No-op before
|
||||||
/// <see cref="StartAsync"/> (storage may be uninitialized and the timer, whose
|
/// <see cref="StartAsync"/> (storage may be uninitialized and the timer, whose
|
||||||
/// presence gates this, is not yet set). Overlap-safe: the sweep's
|
/// presence gates this, is not yet set). Overlap-safe: publishes into
|
||||||
/// <see cref="_retryInProgress"/> CAS makes a concurrent kick a cheap no-op.
|
/// <see cref="_sweepTask"/> only when the CAS is won — see <see cref="KickSweep"/>.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public void TriggerSweep()
|
public void TriggerSweep()
|
||||||
{
|
{
|
||||||
if (_retryTimer == null) return;
|
if (_retryTimer == null) return;
|
||||||
Volatile.Write(ref _sweepTask, RetryPendingMessagesAsync());
|
KickSweep();
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>The current drain handle — test seam for the N3 clobber regression.</summary>
|
||||||
|
internal Task? CurrentSweepTaskForTest => Volatile.Read(ref _sweepTask);
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Starts a sweep IF none is in flight, publishing the new task into
|
||||||
|
/// <see cref="_sweepTask"/> only when this call wins the <see cref="_retryInProgress"/>
|
||||||
|
/// CAS. A kick that loses the CAS returns without touching <see cref="_sweepTask"/> —
|
||||||
|
/// pre-fix it overwrote the drain handle with an instantly-completed no-op, so
|
||||||
|
/// <see cref="StopAsync"/> proceeded with disposal under a still-running sweep
|
||||||
|
/// (review 02 round 2, N3) — defeating the drain exactly when a sweep outlives a tick.
|
||||||
|
/// </summary>
|
||||||
|
private void KickSweep()
|
||||||
|
{
|
||||||
|
if (Interlocked.CompareExchange(ref _retryInProgress, 1, 0) != 0)
|
||||||
|
return;
|
||||||
|
Volatile.Write(ref _sweepTask, RunSweepOwnedAsync());
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Background retry sweep. Processes all pending messages that are due for retry.
|
/// Background retry sweep. Processes all pending messages that are due for retry.
|
||||||
|
/// Self-CASes for direct callers (existing tests); the entry CAS is skipped when
|
||||||
|
/// ownership is already held by <see cref="KickSweep"/>.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
/// <returns>A task representing the asynchronous retry sweep.</returns>
|
/// <returns>A task representing the asynchronous retry sweep.</returns>
|
||||||
internal async Task RetryPendingMessagesAsync()
|
internal async Task RetryPendingMessagesAsync()
|
||||||
@@ -676,7 +696,16 @@ public class StoreAndForwardService
|
|||||||
// Prevent overlapping retry sweeps
|
// Prevent overlapping retry sweeps
|
||||||
if (Interlocked.CompareExchange(ref _retryInProgress, 1, 0) != 0)
|
if (Interlocked.CompareExchange(ref _retryInProgress, 1, 0) != 0)
|
||||||
return;
|
return;
|
||||||
|
await RunSweepOwnedAsync();
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// The actual retry sweep body. The caller MUST already own the
|
||||||
|
/// <see cref="_retryInProgress"/> flag (won the CAS); this method releases it in its
|
||||||
|
/// finally. Never call directly without holding ownership.
|
||||||
|
/// </summary>
|
||||||
|
private async Task RunSweepOwnedAsync()
|
||||||
|
{
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
var gate = _deliveryGate;
|
var gate = _deliveryGate;
|
||||||
|
|||||||
@@ -821,4 +821,67 @@ public class StoreAndForwardServiceTests : IAsyncLifetime, IDisposable
|
|||||||
}
|
}
|
||||||
finally { await service.StopAsync(); }
|
finally { await service.StopAsync(); }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── R2 T8: _sweepTask clobber (N3) ──
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task TriggerSweep_WhileSweepInFlight_DoesNotClobberTheDrainHandle()
|
||||||
|
{
|
||||||
|
var service = CreateService(retryTimerInterval: TimeSpan.FromHours(1));
|
||||||
|
var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||||
|
var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||||
|
service.RegisterDeliveryHandler(StoreAndForwardCategory.ExternalSystem, async _ =>
|
||||||
|
{
|
||||||
|
entered.TrySetResult();
|
||||||
|
await release.Task;
|
||||||
|
return true;
|
||||||
|
});
|
||||||
|
await service.StartAsync();
|
||||||
|
try
|
||||||
|
{
|
||||||
|
await service.EnqueueAsync(StoreAndForwardCategory.ExternalSystem, "t", "{}",
|
||||||
|
attemptImmediateDelivery: false, retryInterval: TimeSpan.Zero);
|
||||||
|
|
||||||
|
service.TriggerSweep(); // real sweep, blocked in the handler
|
||||||
|
await entered.Task.WaitAsync(TimeSpan.FromSeconds(5));
|
||||||
|
service.TriggerSweep(); // redundant kick — pre-fix clobbers _sweepTask
|
||||||
|
|
||||||
|
var handle = service.CurrentSweepTaskForTest;
|
||||||
|
Assert.NotNull(handle);
|
||||||
|
Assert.False(handle!.IsCompleted); // pre-fix: true (a completed no-op replaced the real sweep)
|
||||||
|
}
|
||||||
|
finally
|
||||||
|
{
|
||||||
|
release.TrySetResult();
|
||||||
|
await service.StopAsync();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task StopAsync_WaitsForTheRealInFlightSweep_EvenAfterARedundantTrigger()
|
||||||
|
{
|
||||||
|
var service = CreateService(retryTimerInterval: TimeSpan.FromHours(1));
|
||||||
|
var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||||
|
var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||||
|
service.RegisterDeliveryHandler(StoreAndForwardCategory.ExternalSystem, async _ =>
|
||||||
|
{
|
||||||
|
entered.TrySetResult();
|
||||||
|
await release.Task;
|
||||||
|
return true;
|
||||||
|
});
|
||||||
|
await service.StartAsync();
|
||||||
|
await service.EnqueueAsync(StoreAndForwardCategory.ExternalSystem, "t", "{}",
|
||||||
|
attemptImmediateDelivery: false, retryInterval: TimeSpan.Zero);
|
||||||
|
|
||||||
|
service.TriggerSweep();
|
||||||
|
await entered.Task.WaitAsync(TimeSpan.FromSeconds(5));
|
||||||
|
service.TriggerSweep(); // the clobbering kick
|
||||||
|
|
||||||
|
var stop = service.StopAsync();
|
||||||
|
await Task.Delay(300);
|
||||||
|
Assert.False(stop.IsCompleted); // pre-fix: StopAsync already returned (awaited the no-op)
|
||||||
|
|
||||||
|
release.TrySetResult();
|
||||||
|
await stop.WaitAsync(TimeSpan.FromSeconds(5)); // drains the real sweep promptly once released
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user