agentscope-ai / agentscope-ai/agentscope-java

[Bug]: ReActAgent.stateCache grows unboundedly — no eviction mechanism for long-running singleton HarnessAgent

Abierto
#2,261 1 comentario 0 reacciones 0 asignados Ver en GitHub
area/core/agent bug
Lenguaje dominante
Java
Estrellas
5.6k
Forks
1.3k
Merge medio
4 d 12 h
PR fusionados (30 d)
77

Descripción

## 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.

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.