[Audit][High] ECS query iterator has data race and staleness bug in next()

[MichaelFisher1997]

🔍 Module Scanned\n (automated audit scan)\n\n## 📝 Summary\nThe ECS query iterator's function has two critical concurrency and correctness bugs: (1) the epoch validation check at line 90 is not protected by any lock, creating a data race with concurrent structural modifications, and (2) when the epoch changes, returns without resetting , causing the query to become permanently stale and skip all remaining entities.\n\n## 📍 Location\n- File: \n- Function/Scope: function within \n\n## 🔴 Severity: High\n- Critical: Crashes, data corruption, security vulnerabilities, GPU device loss\n- High: Memory leaks, race conditions, incorrect rendering, broken features\n- Medium: Performance degradation, missing error handling, suboptimal patterns\n- Low: Code style, dead code, minor improvements\n\n## 💥 Impact\nWhen multiple threads use the ECS registry concurrently (e.g., physics system updating on one thread while render system queries on another), the query iterator can return stale or null results even when valid entities exist. Specifically:\n\n1. Data Race: The check at line 90 reads without synchronization, while other threads may be writing to it via , , or . This is a plain data race in Zig (concurrent read and write to same memory).\n\n2. Permanent Staleness: When the epoch check fails, returns but does NOT reset . On the next call to , the same check fails again (epoch still changed), and the query is permanently stuck returning — skipping all remaining entities. This breaks the contract that a query should either return valid rows or indicate completion.\n\n## 🔎 Evidence\n\n\nThe epoch check at line 90 is a data race because:\n- is read without any lock or atomic operation\n- is written in (line 40), (line 48), and (line 56)\n- These writes can happen from any thread using the registry\n\nThe staleness bug occurs because when , the function returns immediately without resetting . If is called again (e.g., after the modifying thread completes), the same epoch check fails again, and the query is stuck.\n\n## 🛠️ Proposed Fix\n\n1. Fix the staleness bug by resetting when the epoch changes so the query can be re-iterated properly:\n\n\n2. Address the data race by either:\n a. Documenting that queries must be completed before any structural modifications (single-threaded iteration), or\n b. Using an atomic load for the epoch check, or\n c. Having the caller explicitly check the epoch before/after iteration\n\nFor the data race, the cleanest fix is to make an and use for the check. Alternatively, document that queries are not thread-safe and must be completed before concurrent // calls.\n\n## ✅ Acceptance Criteria\n- [ ] The function correctly resets when epoch changes, allowing re-iteration\n- [ ] No data race exists between reads in and writes in //\n- [ ] Existing ECS tests in pass\n- [ ] A test verifying query behavior after structural modification is added or existing tests cover this case\n\n## 📚 References\n- Zig language reference on data races: two or more concurrent accesses to the same memory location, at least one being a write\n- Related existing issue: None found covering ECS query staleness\n- Component storage uses which is also not thread-safe for concurrent writes + reads

[github-actions]

The audit accurately identifies two real bugs in modules/engine-ecs/src/manager.zig:

Bug 1 - Data Race (Line 90):

if (self.expected_epoch != self.registry.mutation_epoch) return null;

This reads mutation_epoch without synchronization. Writes occur at lines 40 (create), 48 (destroy), and 56 (clear). This is a plain data race per the Zig language reference.

Bug 2 - Staleness Bug (Line 89-90):
When the epoch check fails, next() returns null immediately without resetting self.index. The query becomes permanently stuck — subsequent calls will fail the same epoch check and skip all remaining entities.

Proposed Fix (correct):

1. Reset index when epoch changes: add self.index = 0; before return null; at line 90
2. For the data race: either make mutation_epoch an std.atomic(u64) with atomic load, or document that queries must complete before structural modifications

Existing tests do NOT cover this. The ECS Query API test at src/ecs_tests.zig:29 only tests single-threaded iteration and wouldn't catch either bug.

Suggested sub-tasks:

1. Fix staleness bug in manager.zig:90 — reset index on epoch mismatch
2. Address data race — either use atomic operations for mutation_epoch or document thread-safety requirements
3. Add test: create entities, start query, modify registry, resume query — verify all valid entities are still returned

opencode session  |  github run