Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ protected override RateLimitLease AttemptAcquireCore(int tokenCount)
// Return SuccessfulLease or FailedLease depending to indicate limiter state
if (tokenCount == 0 && !_disposed)
{
if (_tokenCount > 0)
if (_tokenCount >= 1)
{
Interlocked.Increment(ref _successfulLeasesCount);
return SuccessfulLease;
Expand Down Expand Up @@ -146,7 +146,7 @@ protected override ValueTask<RateLimitLease> AcquireAsyncCore(int tokenCount, Ca
ThrowIfDisposed();

// Return SuccessfulAcquisition if requestedCount is 0 and resources are available
if (tokenCount == 0 && _tokenCount > 0)
if (tokenCount == 0 && _tokenCount >= 1)
{
Interlocked.Increment(ref _successfulLeasesCount);
return new ValueTask<RateLimitLease>(SuccessfulLease);
Expand Down Expand Up @@ -227,7 +227,7 @@ private bool TryLeaseUnsynchronized(int tokenCount, [NotNullWhen(true)] out Rate
ThrowIfDisposed();

// if permitCount is 0 we want to queue it if there are no available permits
if (_tokenCount >= tokenCount && _tokenCount != 0)
if (_tokenCount >= tokenCount && _tokenCount >= 1)
{
if (tokenCount == 0)
{
Expand Down Expand Up @@ -336,7 +336,7 @@ private void ReplenishInternal(long nowTicks)
: queue.DequeueTail();
disposer.Add(nextPendingRequest);
}
else if (_tokenCount >= nextPendingRequest.Count)
else if (_tokenCount >= nextPendingRequest.Count && (nextPendingRequest.Count > 0 || _tokenCount >= 1))
{
Comment on lines +339 to 340
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fulfillment condition works, but it’s hard to read due to redundant comparisons (for Count > 0, _tokenCount >= Count already implies >= 1). Consider rewriting it in a more direct form (e.g., branch on nextPendingRequest.Count == 0) to reduce cognitive load and help avoid future mistakes around the tokenCount == 0 special-case.

Copilot uses AI. Check for mistakes.
// Request can be fulfilled
nextPendingRequest =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,96 @@ public override void AcquireZero_WithoutAvailability()
lease2.Dispose();
}

[Fact]
public void AcquireZero_WithFractionalTokens_Fails()
{
var limiter = new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions
{
TokenLimit = 3,
QueueProcessingOrder = QueueProcessingOrder.OldestFirst,
QueueLimit = 1,
ReplenishmentPeriod = TimeSpan.FromMilliseconds(2),
TokensPerPeriod = 1,
AutoReplenishment = false
});

using var lease1 = limiter.AttemptAcquire(3);
Assert.True(lease1.IsAcquired);
Assert.Equal(0, limiter.GetStatistics()!.CurrentAvailablePermits);

// Replenish a fractional amount (less than 1 token)
// With ReplenishmentPeriod=2ms and TokensPerPeriod=1, replenishing 1ms adds 0.5 tokens
Replenish(limiter, 1);

// Statistics should still report 0 available permits (fractional tokens truncated)
Assert.Equal(0, limiter.GetStatistics()!.CurrentAvailablePermits);

// AttemptAcquire(0) should fail when there are only fractional tokens
var lease2 = limiter.AttemptAcquire(0);
Assert.False(lease2.IsAcquired);
lease2.Dispose();
}

[Fact]
public async Task AcquireAsyncZero_WithFractionalTokens_Queues()
{
var limiter = new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions
{
TokenLimit = 3,
QueueProcessingOrder = QueueProcessingOrder.OldestFirst,
QueueLimit = 1,
ReplenishmentPeriod = TimeSpan.FromMilliseconds(2),
TokensPerPeriod = 1,
AutoReplenishment = false
});

using var lease1 = limiter.AttemptAcquire(3);
Assert.True(lease1.IsAcquired);

Replenish(limiter, 1);
Assert.Equal(0, limiter.GetStatistics()!.CurrentAvailablePermits);

var acquireTask = limiter.AcquireAsync(0);
Assert.False(acquireTask.IsCompleted);

Replenish(limiter, 1);
using var lease2 = await acquireTask;
Assert.True(lease2.IsAcquired);
}
Comment on lines +516 to +522
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this test, awaiting acquireTask without any timeout can hang the test run if the replenishment math/rounding doesn’t quite reach the >= 1 threshold (especially since the test intentionally works with fractional tokens). Consider awaiting with a bounded timeout (e.g., convert to Task and use WaitAsync/WhenAny) and/or replenishing with a larger elapsed time on the second replenish to guarantee completion.

Copilot uses AI. Check for mistakes.

[Fact]
public async Task AcquireAsyncZero_QueuedRequest_NotFulfilledWithFractionalTokens()
{
var limiter = new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions
{
TokenLimit = 3,
QueueProcessingOrder = QueueProcessingOrder.OldestFirst,
QueueLimit = 1,
ReplenishmentPeriod = TimeSpan.FromMilliseconds(4),
TokensPerPeriod = 1,
AutoReplenishment = false
});

using var lease1 = limiter.AttemptAcquire(3);
Assert.True(lease1.IsAcquired);
Assert.Equal(0, limiter.GetStatistics()!.CurrentAvailablePermits);

Replenish(limiter, 2);
Assert.Equal(0, limiter.GetStatistics()!.CurrentAvailablePermits);

var acquireTask = limiter.AcquireAsync(0);
Assert.False(acquireTask.IsCompleted);

Replenish(limiter, 1);
Assert.Equal(0, limiter.GetStatistics()!.CurrentAvailablePermits);
Assert.False(acquireTask.IsCompleted);

Replenish(limiter, 1);
Assert.Equal(1, limiter.GetStatistics()!.CurrentAvailablePermits);
Comment on lines +551 to +552
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion/await sequence is sensitive to timing conversion and truncation: if the final replenishment leaves _tokenCount just under 1.0, CurrentAvailablePermits will still be 0 and the awaited task may not complete. To make the test robust, consider replenishing with a slightly larger elapsed time before asserting/awaiting (or assert availability using a threshold rather than relying on hitting exactly 1.0).

Suggested change
Replenish(limiter, 1);
Assert.Equal(1, limiter.GetStatistics()!.CurrentAvailablePermits);
Replenish(limiter, 2);
Assert.True(limiter.GetStatistics()!.CurrentAvailablePermits >= 1);

Copilot uses AI. Check for mistakes.
using var lease2 = await acquireTask;
Assert.True(lease2.IsAcquired);
}

[Fact]
public override async Task AcquireAsyncZero_WithAvailability()
{
Expand Down
Loading