Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WaitHandles: Possible race condition in InternalWaitOneAsync() #120

Closed
madelson opened this issue Jan 10, 2022 · 1 comment
Closed

WaitHandles: Possible race condition in InternalWaitOneAsync() #120

madelson opened this issue Jan 10, 2022 · 1 comment
Milestone

Comments

@madelson
Copy link
Owner

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 madelson added this to the 2.3.1 milestone Jul 6, 2022
@madelson
Copy link
Owner Author

madelson commented Jul 8, 2022

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
        }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant