agentscope-ai / agentscope-ai/agentscope

[Bug]: Concurrent PATCH sessions causes data loss

Aperta
#2,172 1 commento 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
Python
Stelle
31.5k
Fork
3.5k
Merge medio
1g 23h
PR unite (30g)
95

Descrizione

## Description

The `PATCH /sessions/{session_id}` endpoint implements a read-modify-write pattern without any concurrency control:

```python
# agentscope/app/_router/_session.py
async def update_session(...):
# Step 1: Read existing session
existing = await storage.get_session(user_id, agent_id, session_id)

# Step 2: Merge PATCH fields in memory
config_updates = body.model_dump(exclude_unset=True, exclude={'permission_mode'})
merged_config = {**existing.config.model_dump(mode="json"), **config_updates}

# Step 3: Write back
return await storage.upsert_session(
config=SessionConfig.model_validate(merged_config),
...
)
```

## Problem

When two PATCH requests arrive concurrently (e.g., one updating `name`, another updating `permission_mode`), both read the same old session state, merge their changes, and write back. The second write overwrites the first one's changes.

Example timeline:
```
T1: PATCH-1 reads config -> {name: "新会话", mode: "ask"}
T2: PATCH-2 reads config -> {name: "新会话", mode: "ask"} <- reads stale data
T3: PATCH-1 writes config -> {name: "改名", mode: "ask"}
T4: PATCH-2 writes config -> {name: "新会话", mode: "bypass"} <- overwrites PATCH-1's name
```

## Impact

- Data loss for session config updates
- Affects all storage backends (Redis, SQLite, local files, etc.) — this is NOT storage-specific
- The bug is at the router layer, not the storage layer
- Triggers when clients send concurrent PATCH requests (common in first-message scenarios)
- Critical for distributed deployments (multi-process, multi-instance)

## Root Cause

The router combines multiple storage primitives (get_session + merge + upsert_session) without atomicity guarantee. Storage backends only guarantee single-operation atomicity, not the atomicity of combined business operations.

This is an **application-layer bug** that cannot be solved by changing storage backends.

## Reproduction

1. Create a session
2. Send two concurrent PATCH requests:
```bash
curl -X PATCH ".../sessions/{id}?agent_id=..." -d '{"name":"test1"}' &
curl -X PATCH ".../sessions/{id}?agent_id=..." -d '{"permission_mode":"bypass"}' &
wait
```
3. Check session config — `name` may still be the old value

## Environment

- AgentScope version: 2.0.4.post1 (also affects latest main branch)
- Python version: 3.12
- Affects all storage backends

## Suggested Solutions

### Option 1: Optimistic Locking (Recommended)

Add a `version` field to `SessionRecord` and use CAS (compare-and-swap) during update:

```python
class SessionRecord(BaseModel):
version: int = 1
...

# StorageBase interface
async def cas_session(
self, session_id: str, expected_version: int, new_config: SessionConfig
) -> SessionRecord | None:
"""Compare-and-swap. Returns None if version mismatch."""

# Router
existing = await storage.get_session(...)
updated = await storage.cas_session(
session_id, expected_version=existing.version, new_config=merged_config
)
if updated is None:
raise HTTPException(409, "Session modified, please retry")
```

**Pros**: Storage-agnostic, no external lock service needed, works across processes/instances

**Implementation notes**:
- Redis: use WATCH/MULTI/EXEC for CAS
- SQLite: use `UPDATE ... WHERE version = ?` and check rowcount
- Each storage backend implements `cas_session` accordingly

### Option 2: Distributed Lock

Use Redis lock or database advisory lock keyed by session_id:

```python
# Router
async with distributed_lock(f"session:{session_id}"):
existing = await storage.get_session(...)
# merge and update
```

**Pros**: Explicit mutual exclusion, predictable behavior

**Cons**: Requires lock service (Redis), adds latency, needs timeout handling

Thanks!

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.