David-Desmaisons / RateLimiter

C# rate limiting utility
http://david-desmaisons.github.io/RateLimiter/
MIT License
282 stars 34 forks source link

Deadlock ComposedAwaitableConstraint and Cancellation #27

Open Karql opened 3 years ago

Karql commented 3 years ago

Hi!

At the begining I would like you for great stuff ;)

I've found a bug when using ComposedAwaitableConstraint with Cancellation token.

My use case:

Simplified example:

        private static async Task MainAsync()
        {
            var limiter = TimeLimiter.Compose(
                new CountByIntervalAwaitableConstraint(1, TimeSpan.FromSeconds(1)),
                new CountByIntervalAwaitableConstraint(1000, TimeSpan.FromHours(1))
            );

            Func<int, CancellationToken, Task> f = async (i, token) =>
            {
                await limiter.Enqueue(async () =>
                {
                    await Task.Delay(i * 100);
                    Console.WriteLine(i);
                }, token);
            };

            while (true)
            {
                var cts = new CancellationTokenSource(500);
                var combined = Task.WhenAll(Enumerable.Range(0, 10).Select(i => f(i, cts.Token)));

                try
                {
                    await combined;
                }

                catch
                {
                    ;
                }
            }
        }

Problem is here:

        public async Task<IDisposable> WaitForReadiness(CancellationToken cancellationToken)
        {
            await _Semaphore.WaitAsync(cancellationToken);
            IDisposable[] disposables;
            try 
            {
                disposables = await Task.WhenAll(_AwaitableConstraint1.WaitForReadiness(cancellationToken), _AwaitableConstraint2.WaitForReadiness(cancellationToken));
            }
            catch (Exception) 
            {
                _Semaphore.Release();
                throw;
            } 
            return new DisposeAction(() => 
            {
                foreach (var disposable in disposables)
                {
                    disposable.Dispose();
                }
                _Semaphore.Release();
            });
        }

_AwaitableConstraint2 (for hour) return:

return new DisposeAction(OnEnded);

_AwaitableConstraint1 (for second) throw TaskCancelled exception

Result is that OnEnded wont be never called and _Semaphores wont be released.

I can create new limiter after cancellation but maybe expose method like. TryRelease() on IAwaitableConstraint and call it on exception woluld be better option?

Best regards, Mateusz

David-Desmaisons commented 3 years ago

Could you create a branch with the corresponding test so I can check and debug what is hapenning in this case? Thanks

Karql commented 3 years ago

Hi ;)

https://github.com/Karql/RateLimiter/tree/issue-27

Put break point here: b1

When debugger stops on it put break point in CountByIntervalAwaitableConstraint.WaitForReadiness(CancellationToken cancellationToken) Second call this method show that _Semaphore is closed

b2