WaitHandles: Possible race condition in InternalWaitOneAsync()
madelson opened this issue · 1 comments
madelson commented
Referring to https://github.com/madelson/DistributedLock/blob/master/DistributedLock.WaitHandles/WaitHandleExtensions.cs#L43
Consider the following case:
- CancellationToken fires and sets the task to canceled
- WaitForSingleObject callback runs and fails to set the task
In that case, I think we lose a signal on the handle; ideally if the wait callback fails to set the task then it should re-signal the event.
madelson commented
Reproduction:
[Test]
public async Task TestCancellationDoesNotLeadToLostSignal([Values] bool async)
{
var semaphore = new WaitHandleDistributedSemaphore(nameof(this.TestCancellationDoesNotLeadToLostSignal), 2);
await using var _ = await semaphore.AcquireAsync(TimeSpan.FromSeconds(1));
for (var i = 0; i < 50; ++i)
{
using var barrier = new Barrier(2);
using var source = new CancellationTokenSource();
var acquireTask = Task.Run(async () =>
{
barrier.SignalAndWait();
try
{
if (async) { await using var _ = await semaphore.AcquireAsync(cancellationToken: source.Token); }
else { using var _ = semaphore.Acquire(cancellationToken: source.Token); }
}
catch when (source.Token.IsCancellationRequested) { }
});
var cancelTask = Task.Run(() =>
{
barrier.SignalAndWait();
source.Cancel();
});
await Task.WhenAll(acquireTask, cancelTask);
}
await using var handle = await semaphore.TryAcquireAsync();
Assert.IsNotNull(handle); // if we lost even a single signal due to cancellation in the loop above, this will fail
}