.NET 8 Timer Implementation Has Silent Period Minimum and TokenBucketRateLimiter Is Not Aware of This

Description

We were using TokenBucketRateLimiter with a ReplenishmentPeriod in the microseconds (so less than 1ms).

This value is just passed into the Timer constructor by the limiter.

The timer constructor however, has a line of code where it gets the period in total milliseconds from the timestamp, and then casts it to a long.

This meant that any value < 1ms was being casted to 0. We ran into an issue where the rate limiter was never replenishing, because the timer would not run, so our application would just pause and no errors were present.

In other applications our replenishment periods were > 1ms, so this wasn't caught in those instances.

I don't know if this qualifies as a bug, but at least the documentation should warn you about this. TokenBucketRateLimiterOptions only warns about > TimeSpan.Zero..

Timer's reference docs implicitly mention it here, which is the constructor used by the rate limiter..

These other docs do explicitly mention time being in milliseconds, but it is not the same constructor.

The TokenBucketRateLimiter could maybe do something to prevent the timer from being initialized with a low enough TimeSpan, but then that would couple the limiter implementation much closer to the Timer implementation.

I am not sure how or if this is something the .NET team would look to fix, but for us at least a documentation update or summary comment update would go a long way to make others aware of the limitation.

Reproduction Steps

Create a TokenBucketRateLimiter with a ReplenishmentPeriod of less than 1ms.

        var options = new TokenBucketRateLimiterOptions
        {
            TokenLimit = 1000,
            QueueLimit = int.MaxValue,
            TokensPerPeriod = 1,
            ReplenishmentPeriod = TimeSpan.FromMicroseconds(500),
            AutoReplenishment = true
        };

        var rateLimiter = new TokenBucketRateLimiter(options);

Then in the VS debugger:

Expected behavior

To allow for lower than 1ms timespans or make it explicit that we cannot do a replenishment period (or timer period, depending on where the "fix") should be applied, lower than 1ms unless it's the infinite time span which is allowed.

Actual behavior

Added it in repro steps

Regression?

No response

Known Workarounds

We have our own logic in our applications to ensure

Configuration

.NET 8.0.403 Windows (issue is found in our Linux containers too, running the aspnet image from the Microsoft MCR). We run in x64 but target AnyCPU.

Other information

I am happy to help in any way with fixing this issue, including creating pull requests since I know the team is quite busy. I wanted to consult with you all first before applying a "fix" I would like but that might not be aligned with your goals/thinking.