agentscope-ai / agentscope-ai/agentscope
[Bug]: Concurrent PATCH sessions causes data loss
- 主要語言
- Python
- 星號
- 31.6k
- 分支
- 3.5k
- 平均合併
- 1 天 16 小時
- 30 天內合併 PR
- 103
描述
## 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!
貢獻指南
研究方向
Start in `agentscope/app/_router/_session.py` at `update_session(...)`, where the PATCH read-merge-write flow is implemented, then follow `storage.get_session` and `storage.upsert_session` into the storage interface/backends. Reproduce with the concurrent `PATCH /sessions/{session_id}` example from the issue to confirm the overwrite. Add/inspect session update tests around concurrent PATCH calls (if any exist or create them), and mark done when parallel updates preserve each other’s fields or return conflict-safe behavior without data loss.
由索引模型根據 Issue 內容生成。
評估
- 技術堆疊
- python
- 領域
- api, backend
- Issue 類型
- 缺陷
- 難度
- 4/5
- 預估耗時
- 3-5 天
- 活躍度
- 冷清
- 描述清晰度
- 基本清楚
- 新手友好度
- 56/100