Race condition in RateLimit.enter() allows max_active_requests cap to be exceeded under concurrent requests
- Dominant language
- TypeScript
- Stars
- 156k
- Forks
- 24.6k
- Avg merge
- 22h 9m
- Merged PRs (30d)
- 610
Description
### Self Checks
- [x] I have read the [Contributing Guide](https://github.com/langgenius/dify/blob/main/CONTRIBUTING.md) and [Language Policy](https://github.com/langgenius/dify/issues/1542).
- [x] This is only for bug report, if you would like to ask a question, please head to [Discussions](https://github.com/langgenius/dify/discussions/categories/general).
- [x] I have searched for existing issues [search for existing issues](https://github.com/langgenius/dify/issues), including closed ones.
- [x] I confirm that I am using English to submit this report, otherwise it will be closed.
- [ ] 【中文用户 & Non English User】请使用英语提交,否则会被关闭 :)
- [x] Please do not modify this template :) and fill in all the required fields.
### Dify version
1.16.0-rc1 (also present on `main`)
### Cloud or Self Hosted
Self Hosted (Source)
### Steps to reproduce
`RateLimit.enter()` (`api/core/app/features/rate_limiting/rate_limit.py:81-87`) is meant to cap the number of concurrently active requests per app to `max_active_requests`, using a Redis hash (`active_requests_{client_id}`) where each in-flight request holds one field.
```python
def enter(self, request_id: Optional[str] = None) -> str:
if self.disabled():
return request_id or RateLimit._unlimited_request_id
if not request_id:
request_id = RateLimit.gen_request_key()
if redis_client.hlen(self.active_requests_key) >= self.max_active_requests:
raise AppInvokeQuotaExceededError(...)
redis_client.hset(self.active_requests_key, request_id, str(time.time()))
return request_id
```
The check (`hlen(...) >= max_active_requests`) and the write (`hset(...)`) are two separate, unsynchronized Redis operations — this is a classic check-then-act race. If N requests for the same `client_id` arrive concurrently while the hash is at `max_active_requests - 1`, all of them can execute `hlen` before any of them executes `hset`, so all N see a count below the cap and all N proceed to `hset`, pushing the active count to `max_active_requests - 1 + N`. There's no atomic primitive (e.g. a Lua script, `WATCH`/`MULTI`, or `HSETNX` combined with a separate counter) enforcing the cap across the two calls.
### ✔️ Expected Behavior
The number of concurrently active requests per app should never exceed `max_active_requests` — the check-and-increment should be atomic, e.g. via a Redis Lua script or an atomic counter (`HINCRBY`-style pattern) that enforces the cap in a single round trip.
### ❌ Actual Behavior
Under concurrent load, more than `max_active_requests` requests can be admitted simultaneously, defeating the purpose of the rate limiter. This is a self-inflicted capacity/DoS-style issue (a burst of legitimate concurrent requests from the app's own users can overwhelm backend capacity that the limiter was supposed to protect), not a cross-tenant security issue.
Contributor guide
Research direction
Start with RateLimit.enter() in api/core/app/features/rate_limiting/rate_limit.py:81-87 and examine the separate Redis hlen and hset operations. Reproduce concurrent requests when the hash is at max_active_requests - 1, then ensure the check and write enforce the cap atomically without admitting excess requests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, redis
- Domain
- backend, distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 58/100