langgenius / langgenius/dify

Race condition in RateLimit.enter() allows max_active_requests cap to be exceeded under concurrent requests

Open
#39,177 2 comments 1 reaction 0 assignees View on GitHub
🐞 bug 1.16.0 project#dify
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.