Skip to content

AgentRegistry exposes and enumerates a mutable Dictionary outside its lock #68

@nficano

Description

@nficano

Summary

Second-pass well-written-code audit finding. AgentRegistry.AgentEntry protects writes to Versions with _gate, but exposes the mutable Dictionary publicly and ToInventory() enumerates it without taking that same lock.

Evidence

  • src/Arcp.Runtime/Agents/AgentRegistry.cs:65 builds inventory from _byName.Values.
  • src/Arcp.Runtime/Agents/AgentRegistry.cs:69 reads e.Versions.Count and enumerates e.Versions.Keys.ToArray() without locking AgentEntry._gate.
  • src/Arcp.Runtime/Agents/AgentRegistry.cs:79 exposes public Dictionary<string, IAgent> Versions.
  • src/Arcp.Runtime/Agents/AgentRegistry.cs:92 through src/Arcp.Runtime/Agents/AgentRegistry.cs:98 mutates that dictionary under _gate in AddVersion.

Why it matters

Concurrent registration and handshakes/inventory generation can enumerate a Dictionary while it is being mutated. That can throw InvalidOperationException, produce inconsistent welcome inventory, and makes the locking discipline hard to reason about.

Proposed fix

Hide Versions behind snapshot methods on AgentEntry, e.g. SnapshotInventoryFields() and TryGetVersion(), and ensure all dictionary reads/writes happen under _gate. Alternatively replace the mutable dictionary with immutable snapshots swapped atomically.

Acceptance criteria

  • No public/internal API exposes the mutable Dictionary instance.
  • ToInventory() obtains a consistent per-agent snapshot without enumerating unlocked mutable state.
  • A concurrent test repeatedly calls RegisterVersion and ToInventory without exceptions or partial state.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions