[Bug]: Per-path lock maps grow without bound — no eviction in SkillBox, WorkspaceManager and LocalFilesystem

[gxgeek-n]

Summary

Three lock maps are keyed by file path and never evict entries. In an agent deployment the paths are chosen by the model rather than by a fixed application config, so the key space is effectively unbounded.

Related to #2261 (ReActAgent.stateCache unbounded growth), but these are different maps and one of them is static.

Affected code (current main)

Location	Declaration	Key	Lifetime
agentscope-core/.../skill/SkillBox.java:57	private static final ConcurrentHashMap<String, Object> FILE_LOCKS	targetPath.toString() (line 834)	JVM-wide, shared by every SkillBox instance
agentscope-harness/.../workspace/WorkspaceManager.java:116	private final Map<String, ReentrantLock> pathLocks	normalized / relative path (lines 376, 409, 456, 646)	instance
agentscope-harness/.../filesystem/local/LocalFilesystem.java:93	private final ConcurrentHashMap<String, ReentrantLock> fileLocks	lock key (line 359)	instance

Each is populated exclusively through computeIfAbsent, e.g.:

// SkillBox.java:834
Object lock = FILE_LOCKS.computeIfAbsent(targetPath.toString(), k -> new Object());

Grepping each file for remove / clear on these maps returns 0 matches — entries are only ever added.

Why it matters here

FILE_LOCKS being static is the notable one: every distinct path any skill has ever written to retains an entry for the lifetime of the process, across all SkillBox instances.

The usual justification for never evicting a per-key lock map is that the key space is bounded by application configuration. That does not hold for an agent runtime — file paths come from model output, so a long-running process accumulates one entry per distinct path the model ever produced.

Scale, stated honestly

The leaked values are small (Object, ReentrantLock), so this is a slow leak rather than a fast one — unlike #2261, which retains AgentState objects. Reaching memory pressure needs a large number of distinct paths. I am reporting it as a boundedness defect rather than claiming an imminent OOM.

Possible directions

1. Striped locking — a fixed number of locks selected by key.hashCode(), so the map never grows (the standard fix; Guava's Striped does exactly this). Costs occasional false sharing between unrelated paths.
2. Remove after release — finally { map.remove(key); }, but that races with another thread that is about to acquire the same lock, so it needs care.
3. Bounded cache with eviction — matches whatever approach [Bug]: ReActAgent.stateCache grows unboundedly — no eviction mechanism for long-running singleton HarnessAgent #2261 settles on, keeping the codebase consistent.

I have not opened a PR because the choice among these is a design call, and option 3 in particular should probably follow whatever #2261 lands on. Happy to implement whichever direction maintainers prefer, with a test asserting the map stays bounded across many distinct paths.

[oss-maintainer]

Verified. All three maps (SkillBox.FILE_LOCKS, WorkspaceManager.pathLocks, LocalFilesystem.fileLocks) use computeIfAbsent with path-derived keys and never evict.

The static modifier on SkillBox.FILE_LOCKS makes it the most concerning — it's JVM-wide and shared across every SkillBox instance. In an agent runtime where paths come from model output, the key space is effectively unbounded.

Classification: Bug (boundedness defect). The leak is slow (small lock objects), but for long-running deployments it's a real memory concern.

Suggested approach: Option 1 (striped locking) is the simplest and most appropriate here — fixed memory footprint, no eviction races, and the false-sharing cost is negligible for file I/O workloads. Option 3 (bounded cache) would be consistent with whatever lands for #2261 (stateCache eviction), but adds complexity that may not be warranted for lock objects.

Flagging for maintainer decision on the approach. Happy to see a PR for whichever direction is chosen.

[liulangjietou]

I'd like to work on this one, following the suggested direction (Option 1, striped locking).

Plan:

• Introduce a small internal fixed-size lock-striping utility (locks selected by key.hashCode()), so memory footprint is constant regardless of how many distinct paths the model produces.
• Apply it to the three sites while keeping each site's current locking semantics unchanged: SkillBox.FILE_LOCKS (Object monitor), WorkspaceManager.pathLocks and LocalFilesystem.fileLocks (ReentrantLock).
• Add a unit test asserting the lock structures stay bounded across a large number of distinct paths, plus a concurrency test verifying mutual exclusion per key still holds.

@gxgeek-n you mentioned you'd be happy to implement — if you were already planning to pick this up, it's yours and I'll step back. Otherwise I'll open a focused PR along the lines above.