agentscope-ai / agentscope-ai/agentscope-java

[Bug]: Concurrent calls on one agent corrupt each other's sandbox binding (single-slot `SandboxBackedFilesystem` / `SandboxLifecycleMiddleware`)

Open
#2,490 0 comments 0 reactions 0 assignees View on GitHub
area/build area/core/agent area/core/memory area/extensions area/harness bug
Dominant language
Java
Stars
5.6k
Forks
1.3k
Avg merge
4d 12h
Merged PRs (30d)
77

Description

**Describe the bug**

`SandboxBackedFilesystem` and `SandboxLifecycleMiddleware` both assume that a given agent instance runs **at most one call at a time**, and store per-call sandbox state in **agent-level single slots**:

- `SandboxBackedFilesystem` is a stable proxy created at agent build time and shared by all tool executions of the agent. The live sandbox is injected into a single field:

```java
// agentscope-harness/.../filesystem/sandbox/SandboxBackedFilesystem.java
private volatile Sandbox sandbox; // injected per call, cleared on release
```

- `SandboxLifecycleMiddleware` keeps the acquire result of "the current call" in a single reference:

```java
// agentscope-harness/.../middleware/SandboxLifecycleMiddleware.java
private final AtomicReference currentAcquireResult =
new AtomicReference<>();
```

When the same agent bean is called concurrently for **different sessions** (a common setup: one shared `ReActAgent`/`HarnessAgent` bean serving many users/conversations — the SDK's `serializeOnKey` only serializes calls of the *same* `(userId, sessionId)`, cross-session calls run in parallel by design), the two slots are corrupted in three distinct ways:

1. **Sandbox cross-talk** — call B's `filesystemProxy.setSandbox(sbB)` overwrites call A's `sbA` while A is still running. All subsequent filesystem/tool operations of A silently land in **B's sandbox**: files leak across sessions, workspace projection of one conversation is visible to another.

2. **Spurious `No active sandbox` failures** — when A finishes first, `releaseForCall` executes `filesystemProxy.setSandbox(null)`. B is still mid-run; its next filesystem operation throws:

```
io.agentscope.harness.agent.sandbox.SandboxException$SandboxConfigurationException:
No active sandbox — sandbox filesystem used outside of a call context
```

3. **Wrong-pair release + sandbox session leak** — `releaseForCall` does `currentAcquireResult.getAndSet(null)`. If B's acquire has overwritten the slot, A's release will `persistState`/`release` **B's** `SandboxAcquireResult` (stopping the sandbox B is actively using), while A's own acquire result is lost and its sandbox session is **never released**.

We hit (2) in production at scale: an agent that dispatches IM (DingTalk) group messages across many conversations had a large fraction of runs fail with `No active sandbox` whenever two conversations were active at once.

**To Reproduce**

1. Build one `ReActAgent` (harness) with a sandbox-backed filesystem and any filesystem tool (e.g. `execute`).
2. Fire two concurrent `call()`s with **different** `sessionId`s (so `serializeOnKey` does not serialize them), each running a tool loop that touches the filesystem for a few seconds.
3. Observe: `SandboxConfigurationException: No active sandbox` on the longer-running call once the shorter one completes; with logging on the sandbox side, also observe call A's commands executing inside call B's sandbox session before that.

**Expected behavior**

Each in-flight call operates on **its own** sandbox for its entire duration, regardless of concurrent calls on the same agent instance. Acquire/release must be paired per call: a finishing call must never clear or release another call's sandbox.

**Error messages**

```
io.agentscope.harness.agent.sandbox.SandboxException$SandboxConfigurationException:
No active sandbox — sandbox filesystem used outside of a call context
at ...SandboxBackedFilesystem.requireSandbox(SandboxBackedFilesystem.java)
at ...SandboxBackedFilesystem.execute(SandboxBackedFilesystem.java)
```

**Proposed fix: bind the sandbox to the per-call `RuntimeContext` instead of agent-level slots**

The preconditions are already in place:

- Every `SandboxBackedFilesystem` operation (`execute` / `uploadFiles` / `downloadFiles`) already receives the per-call `RuntimeContext` as a parameter.
- `RuntimeContext` already provides a typed attribute container (`ctx.get(Class)` / `ctx.put(Class, T)`).

Concrete changes:

1. **Inject side** — in `SandboxLifecycleMiddleware.acquireForCall(ctx)`, stop writing agent-level slots; store per call instead:

```java
ctx.put(Sandbox.class, sandbox);
ctx.put(SandboxAcquireResult.class, result);
```

2. **Use side** — change `SandboxBackedFilesystem.requireSandbox()` to `requireSandbox(RuntimeContext ctx)` resolving via `ctx.get(Sandbox.class)`; delete the `volatile Sandbox sandbox` field. Deprecate (or re-shape) the `SandboxAware` setter/getter accordingly.

3. **Release side** — `releaseForCall(ctx)` retrieves **its own** `SandboxAcquireResult` from `ctx` for `persistState`/`release`/`lease.close()`; delete `currentAcquireResult`.

This removes all three failure modes at once: no overwrite (each call holds its own reference), no spurious null-out (release only clears its own context entry), no wrong-pair release (acquire/release are naturally paired through the ctx object). Thread-safety across Reactor/Netty thread hops comes for free because the reference travels with the per-call `RuntimeContext`.

Follow-ups to consider in the same change:

- Audit call paths into the sandbox filesystem that pass a `null`/shared `RuntimeContext` — any such path is a latent bug and should be fixed alongside.
- `SandboxAware#setSandbox/getSandbox` is public API; keep a deprecated no-op for one release if needed.
- Keep the `SandboxExecutionGuard` extension point — it remains useful for genuine concurrency quotas (e.g. max sandboxes per user), it just should no longer be needed as a "serialize everything" workaround.

**Alternatives considered**

- *Keyed map inside the proxy (callId → Sandbox)*: the use side has no reliable callId without ThreadLocal, and the execution hops across Reactor/Netty threads where ThreadLocal propagation breaks — fragile.
- *Per-call filesystem instances*: tools are bound to the filesystem proxy at agent build time; making that per-call would ripple through tool registration — far more invasive than the `RuntimeContext` binding.

**Current workaround (for other users hitting this)**

Implement a `SandboxExecutionGuard` with a fair single-permit `Semaphore` that serializes *all* calls of the agent (`tryEnter` → acquire, lease close → release). Note: use a `Semaphore`, **not** `ReentrantLock` — the lease is closed by the SDK during `Mono.using` cleanup on a Reactor/Netty thread different from the acquiring thread, and a cross-thread `unlock()` throws `IllegalMonitorStateException`, permanently leaking the lock. Cost of the workaround: cross-session calls degrade from parallel to FIFO queueing on that agent.

**Environment (please complete the following information):**

- AgentScope-Java Version: 2.0.0 (agentscope-core / agentscope-harness)
- Java Version: 17+
- OS: linux (production), macos (dev) — OS-independent

**Additional context**

Related (same concurrency assumption, previously reported separately): the session slot key `userId + "/" + sessionId` is parsed back via `lastIndexOf('/')`, which mis-splits when `sessionId` itself contains `/` (e.g. base64 DingTalk `openConversationId`).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.