fix(terminal): acquire _env_lock for all shared dict accesses

[aaronlab]

Summary

• Acquire _env_lock before accessing _active_environments and _last_activity in four functions that previously read/iterated these dicts without the lock, risking RuntimeError: dictionary changed size during iteration when the background cleanup thread concurrently pops entries.

Details

get_active_environments_info() (line 938)

Previously iterated _active_environments directly without _env_lock. If _cleanup_inactive_envs() calls .pop() on the dict via the background cleanup thread during iteration, Python raises RuntimeError. Fix: snapshot count and task_ids under the lock, then iterate the snapshot for disk-usage calculation (the slow rglob call stays outside the lock).

cleanup_all_environments() (line 963)

Read .keys() without the lock. While list() makes the snapshot itself safe, the pattern is inconsistent with every other function in the module. Fix: acquire _env_lock for the snapshot.

_cleanup_inactive_envs() pre-scan (line 822)

Read and wrote _last_activity without the lock, racing with terminal_tool() which updates _last_activity inside the lock (lines 1205, 1222, 1280). Fix: snapshot keys under lock, write updates under lock.

_atexit_cleanup() (line 1032)

Accessed _active_environments directly for truthiness check and len(). Fix: read len() under lock.

Test plan

• Verify get_active_environments_info() returns correct data under concurrent cleanup
• Verify _cleanup_inactive_envs pre-scan doesn't race with terminal_tool activity updates
• Run pytest tests/ -q to ensure no regressions

🤖 Generated with Claude Code

[trevorgordon981]

The key snapshots before iteration (lines 824-825) and the locking around reads/writes (lines 943-946, 969-970, 1040-1041 in ) look correct for preventing race conditions in this file.

Have you checked whether other parts of the codebase interact with these same shared resources (, ) to ensure consistent locking patterns everywhere? Also, considering the critical nature of these locks, have you thought about adding specific concurrency tests (e.g., stress tests with multiple threads) to validate that these locks hold up under heavy load?

[alt-glitch]

Overlaps significantly with #6684 which bundles this same _env_lock fix along with two other fixes (dead regex, _write_count race).

[alt-glitch]

Overlaps significantly with #6684 which bundles this same _env_lock fix along with two other fixes.

[teknium1]

Thanks for identifying a real lifecycle-locking gap. The premise still holds on current main: tools/terminal_tool.py:1550-1552, :1669, and :1765-1771 access the shared environment dictionaries outside _env_lock while cleanup_vm() removes entries under the lock at :1719-1721.

Problems

• get_active_environments_info() no longer exists on current main; the corresponding hunk should be dropped during salvage.
• Current _atexit_cleanup() has an additional list(_active_environments.values()) snapshot at tools/terminal_tool.py:1771. The PR locks only the count, leaving this new snapshot unlocked.
• The PR has no concurrency regression test for these paths.

Suggested changes

• Adapt the remaining lock changes to current tools/terminal_tool.py.
• Snapshot both count and envs_to_wait under _env_lock in _atexit_cleanup(), then perform cleanup and waits outside the lock.
• Add a deterministic threaded test that races cleanup_vm() with each snapshot path.

Automated hermes-sweeper review.

[teknium1]

Current main's _atexit_cleanup() also snapshots list(_active_environments.values()) before cleanup. Include that snapshot in this same _env_lock block; otherwise the current HEAD path can still race with cleanup_vm().