agentscope-ai / agentscope-ai/agentscope-java
[Bug]: ReActAgent.stateCache grows unboundedly — no eviction mechanism for long-running singleton HarnessAgent
- 主要言語
- Java
- スター
- 5.6k
- フォーク
- 1.3k
- 平均マージ
- 4日 12時間
- マージ済み PR(30日)
- 77
説明
## Summary
`ReActAgent.stateCache` (and `permissionEngineCache`) is a plain `ConcurrentHashMap` with **no TTL, no LRU, no eviction, and no cleanup API**. When `HarnessAgent` is used as the documented singleton serving many `(userId, sessionId)` slots, both maps grow monotonically for the lifetime of the JVM — eventually causing OOM in any long-running multi-tenant deployment.
This is especially concerning because the [official HarnessAgent Javadoc](https://github.com/agentscope-ai/agentscope-java/blob/main/agentscope-harness/src/main/java/io/agentscope/harness/agent/HarnessAgent.java#L147-L150) explicitly recommends the singleton pattern:
> **Thread Safety:** `HarnessAgent` is stateless between calls and safe to use as a singleton serving multiple users/sessions concurrently. Each `call()` uses the `RuntimeContext`'s `(userId, sessionId)` to isolate state.
So the recommended usage pattern directly amplifies the leak.
## Evidence
### 1. `stateCache` / `permissionEngineCache` are plain `ConcurrentHashMap`
https://github.com/agentscope-ai/agentscope-java/blob/main/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java#L267-L274
```java
private final ConcurrentHashMap stateCache = new ConcurrentHashMap<>();
private final ConcurrentHashMap permissionEngineCache = new ConcurrentHashMap<>();
```
### 2. Write paths — only `put` / `computeIfAbsent`, never `remove` / `clear`
```bash
$ grep -n "stateCache\.\|permissionEngineCache\." agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java
456: stateCache.put(slot, loaded);
459: stateCache.computeIfAbsent(slot, ...);
472: permissionEngineCache.put(slot, loadedEngine);
475: permissionEngineCache.computeIfAbsent(slot, ...);
3644: return stateCache.computeIfAbsent(slot, ...);
3680: permissionEngineCache.put(slot, new PermissionEngine(...));
3775: return permissionEngineCache.computeIfAbsent(slot, ...);
```
**No `remove`, `clear`, `invalidate`, `evict` calls anywhere in the file.** Same across the entire `agentscope-core` and `agentscope-harness` modules.
### 3. `close()` is a no-op
```java
// ReActAgent.java#L3822-L3826
@Override
public void close() {
// No-op for the core ReActAgent. Subclasses / wrappers (HarnessAgent) may release
// additional resources here.
}
```
No cache cleanup happens even when the agent is closed.
### 4. `activateSlotForContext` puts unconditionally on every `call()`
https://github.com/agentscope-ai/agentscope-java/blob/main/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java#L442-L483
```java
private CallExecution activateSlotForContext(RuntimeContext ctx) {
// ...
if (stateStore != null) {
loaded = loadOrCreateAgentStateForSlot(stateStore, finalUid, finalSid, ...);
stateCache.put(slot, loaded); // ⚠️ always puts, never evicts
} else {
loaded = stateCache.computeIfAbsent(slot, k -> ...); // ⚠️ also grows unboundedly
}
// ... permissionEngineCache mirrors the same pattern
}
```
When `stateStore != null` (the distributed-production case), every `call()` re-hydrates from the DB **and** writes back into the in-memory map. The map becomes an ever-growing mirror of every session the agent has ever seen.
## Impact
### Memory usage over time
Assuming each `AgentState` ~10 KB (typical with conversation buffer) and each `PermissionEngine` ~2 KB:
| Sessions seen | `stateCache` memory | `permissionEngineCache` | Total |
|---|---|---|---|
| 1,000 | ~10 MB | ~2 MB | ~12 MB |
| 10,000 | ~100 MB | ~20 MB | ~120 MB |
| 100,000 | ~1 GB | ~200 MB | ~1.2 GB |
| 1,000,000 | ~10 GB | ~2 GB | ~12 GB 💥 |
For a Spring Boot service running `HarnessAgent` as a singleton for months (the intended pattern), this **will** OOM.
### Distributed semantics hazard
Because `stateCache` is a JVM-local mirror:
1. Pod A loads `user1/session1` → `stateCache["user1/session1"] = state_v1`
2. Pod B loads the same slot → `stateCache["user1/session1"] = state_v1`
3. Pod A processes a call, mutates to `state_v2`, saves to DB
4. Pod B's cache still holds `state_v1` (stale)
5. Pod B processes the next call on the same slot, writes `state_v3` based on `state_v1` → **`state_v2` is silently lost**
Without either (a) sticky sessions, (b) optimistic locking on `AgentState`, or (c) a cross-Pod invalidation channel, this is a real correctness risk.
## Reproduction
```java
// Long-running singleton HarnessAgent, the documented production pattern
HarnessAgent agent = HarnessAgent.builder()
.model(model)
.stateStore(new MySqlAgentStateStore(dataSource)) // distributed store
// ...
.build();
// Many sessions over hours/days
for (int i = 0; i < 1_000_000; i++) {
agent.call("hello", RuntimeContext.builder()
.userId("user-" + (i % 10_000))
.sessionId("session-" + i)
.build()).block();
}
// Inspect: no way to observe, no way to evict.
// The agent now holds 1M AgentState instances in memory.
```
## Proposed Fix
### Primary: replace with Caffeine `Cache`
```java
private final Cache stateCache = Caffeine.newBuilder()
.maximumSize(50_000)
.expireAfterAccess(Duration.ofMinutes(30))
.recordStats()
.build();
```
`Caffeine` gives us:
- Hard size cap (`maximumSize`) — bounds memory even under pathological load
- Idle eviction (`expireAfterAccess`) — sessions not touched for N minutes are dropped
- `RemovalListener` — optional logging / metrics on eviction
- `stats()` — hit/miss/eviction metrics for monitoring
`permissionEngineCache` should receive the same treatment.
### Secondary: management API
```java
// Evict a specific session (e.g. on user logout / session delete)
public void evictSession(String userId, String sessionId) {
String slot = slotKey(userId, sessionId);
stateCache.invalidate(slot);
permissionEngineCache.invalidate(slot);
}
// Evict all sessions for a user
public void evictUser(String userId) {
String prefix = (userId == null ? "__anon__" : userId) + "/";
stateCache.asMap().keySet().removeIf(k -> k.startsWith(prefix));
permissionEngineCache.asMap().keySet().removeIf(k -> k.startsWith(prefix));
}
// Expose for Actuator / observability
public CacheStats getCacheStats() { return stateCache.stats(); }
```
### Tertiary: `close()` should clean up
```java
@Override
public void close() {
stateCache.invalidateAll();
permissionEngineCache.invalidateAll();
}
```
### Optional: optimistic locking for distributed safety
Add a `version` field on `AgentState` and a `saveIfVersionMatches(userId, sessionId, key, value, expectedVersion)` method on `AgentStateStore`. This protects against the Pod-A/Pod-B overwrite scenario described above even when sticky sessions aren't configured.
## Alternatives Considered
1. **Per-request `HarnessAgent` creation** — the `ReActAgent` Javadoc mentions this as a workaround, but:
- `HarnessAgent` is expensive to build (workspace manager, sandbox context, skill curator, plan mode manager, MCP registration, middleware chain, …)
- The HarnessAgent Javadoc explicitly recommends singleton usage
- So this is not a viable production workaround for the intended use case
2. **Periodic JVM restarts** — obviously not a real solution for a production framework
3. **Weak/Soft references** — would allow GC-driven eviction but lose control over timing and make behavior unpredictable under memory pressure; Caffeine's explicit policies are a much better fit
## Environment
- AgentScope-Java Version: 2.0.x (current `main`)
- Java Version: 17+
- Affects: any long-running `HarnessAgent` singleton with a non-null `AgentStateStore` — i.e. every production deployment of `agentscope-paw` / `agentscope-builder` / custom Spring Boot integrations serving multiple users
## Additional Context
- `agentscope-paw` and `agentscope-builder` both construct a single `HarnessAgent` at startup that serves all incoming requests — exactly the singleton pattern the Javadoc recommends, and exactly the pattern that triggers this issue.
- There is currently **no public API** on `ReActAgent` or `HarnessAgent` to observe cache size, trigger eviction, or hook into any cleanup lifecycle.
- Adding Caffeine as a dependency is lightweight (~300 KB) and is already the de-facto standard for in-process caching in the Java ecosystem.
コントリビューションガイド
評価
この issue はまだ評価されていません。