Rate limiting sensible to race conditions

[sadraskol]

Hi there! We've identified a potential race condition when counting down rate limits. Let me explain:

List of actions

There are two actions that can are atomically executed for each request:

• GET key: returns the current remaining count as a usize
• DECRBY key 1: decrement the counter by 1

Say there's only 1 request left for the rate limiting. If two (probably more) requests are issued concurrently the following can happen:

• req A: Get key -> returns 1
• req B: Get key -> returns 1
• req A: DECRBY key 1 -> key = 0
• req B: DECRBY key 1 -> key = -1

What now?

The current code only deals with usize counters, so the behavior depends on the implementation of the stores, but overall I think the code is too optimistic for this scenario.

One solution would be to use an atomically safe Get + Decr, using locks for instance. But I think the cost is too high for the normal case.

I would rather document the possibility of this scenarion and I would recommend 2 changes to the current implementation:

• use i32 instead of usize to express the counter to acknowledge this scenario
• make sure the code checks that remaining <= 0 instead of remaining == 0

If you agree, I can contribute to make these changes. I wanted to make sure we agree and we didn't forget anything before moving on.