ejfinneran / ratelimit

A Redis-backed rate limiter written in Ruby
MIT License
257 stars 55 forks source link

Fix potential race condition in #add #20

Closed jeremywadsack closed 8 years ago

jeremywadsack commented 8 years ago

When incrementing the counter, we should use Redis#multi, rather than Redis#pipeline, to ensure that the hincrby, hdel, and expire are in the same call. If another process is checking #count between when hincrby and expire are called it could lead to an invalid count. Redis#multi protects against that.

See the first example for rate limiting in the Redis documentation for INCR. Also note that the JS code that this gem was inspired by used the multi/exec pattern as well.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 99.138% when pulling 789f2c800db00c5fccc8cc30caab140d8076cdf3 on jeremywadsack:race_condition into aa8bd7a71e263a86451a75216589d408fa133342 on ejfinneran:master.

ejfinneran commented 8 years ago

This is great. Thanks!