[API Proposal]: System.Threading.RateLimiting RateLimitLease should implement IAsyncDisposable

[cristipufu]

Background and motivation

Hi, I'm working on a Redis extension library on top of Microsoft.AspNetCore.RateLimiting (for multi-node deployments), using the new Rate Limiting support added in NET 7: https://github.com/cristipufu/aspnetcore-redis-rate-limiting

When the ConcurrencyLimiter's ConcurrencyLease gets disposed, we need to basically decrement the count for that particular policy. At the moment, the current API forces me to use the sync version.

My suggestion would be to make RateLimitLease implement IAsyncDisposable, such that we can call async APIs when the lease is released: https://github.com/cristipufu/aspnetcore-redis-rate-limiting/blob/master/src/RedisRateLimiting/Concurrency/RedisConcurrencyRateLimiter.cs#L114

API Proposal

namespace System.Threading.RateLimiting
{
    public abstract class RateLimitLease : IDisposable, IAsyncDisposable
    {
        ...

       protected virtual ValueTask DisposeAsync(bool disposing)
       {
           return default;
       }

        public ValueTask DisposeAsync()
        {
            return DisposeAsync(true);
        }
    }
}

API Usage

private sealed class RedisConcurrencyLease : RateLimitLease
{
     ...

    protected override ValueTask DisposeAsync(bool disposing)
    {
        if (_disposed)
        {
            return;
        }

        _disposed = true;

        if (_id != null)
        {
            return _limiter?.ReleaseAsync(_id);
        }

       return default;
    }
}

Alternative Designs

No response

Risks

No response

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

Hi, I'm working on a Redis extension library on top of Microsoft.AspNetCore.RateLimiting (for multi-node deployments), using the new Rate Limiting support added in NET 7: https://github.com/cristipufu/aspnetcore-redis-rate-limiting

When the ConcurrencyLimiter's ConcurrencyLease gets disposed, we need to basically decrement the count for that particular policy. At the moment, the current API forces me to use the sync version.

My suggestion would be to make RateLimitLease implement IAsyncDisposable, such that we can call async APIs when the lease is released: https://github.com/cristipufu/aspnetcore-redis-rate-limiting/blob/master/src/RedisRateLimiting/Concurrency/RedisConcurrencyRateLimiter.cs#L114

API Proposal

namespace System.Threading.RateLimiting
{
    public abstract class RateLimitLease : IDisposable, IAsyncDisposable
    {
        ...

       protected virtual ValueTask DisposeAsync(bool disposing)
       {
           return default;
       }

        public ValueTask DisposeAsync()
        {
            return DisposeAsync(true);
        }
    }
}

API Usage

private sealed class RedisConcurrencyLease : RateLimitLease
{
     ...

    protected override ValueTask DisposeAsync(bool disposing)
    {
        if (_disposed)
        {
            return;
        }

        _disposed = true;

        if (_id != null)
        {
            return _limiter?.ReleaseAsync(_id);
        }

       return default;
    }
}

Alternative Designs

No response

Risks

No response

Author:	cristipufu
Assignees:	-
Labels:	api-suggestion, area-System.Threading, untriaged
Milestone:	-

[cristipufu]

This one should be async as well: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/RateLimiter.cs#L37

[cristipufu]

Tagging @BrennanConroy @davidfowl @halter73 @JamesNK for visibility
Please take a look: cristipufu/aspnetcore-redis-rate-limiting#66