[Audit][Medium] Input state hash maps become inconsistent when keys_pressed.put() allocation fails

[MichaelFisher1997]

🔍 Module Scanned

modules/engine-input/src/ (automated audit scan)

📝 Summary

When keys_pressed.put() fails due to allocation error, a key is added to keys_down but not keys_pressed, creating an inconsistent state where isKeyDown returns true but isKeyPressed returns false for the same key. This breaks the InputMapper's isActionActive logic which checks both held and pressed states.

📍 Location

• File: modules/engine-input/src/input.zig:100-104
• Function/Scope: processEvent / SDL_EVENT_KEY_DOWN handling

🔴 Severity: Medium

• Medium: Missing error handling leading to state inconsistency in edge case

💥 Impact

If keys_pressed.put() fails (e.g., OOM), a key appears held (isKeyDown=true) but never registered as pressed (isKeyPressed=false). This breaks InputMapper.isActionActive checks that rely on isKeyPressed, causing held keys to fail action triggers that depend on press detection. The key also becomes "stuck" in keys_down since the next release event will attempt to remove it again.

🔎 Evidence

c.SDL_EVENT_KEY_DOWN => {
    if (!event.key.repeat) {
        const key = Key.fromSDL(event.key.key);
        self.keys_down.put(key, {}) catch {};        // Line 102 - succeeds
        self.keys_pressed.put(key, {}) catch {};      // Line 103 - FAILS, key not in keys_pressed
    }
},

If line 103's put fails, the key is in keys_down (line 102 succeeded) but NOT in keys_pressed. Result: isKeyDown=true, isKeyPressed=false for the same key — inconsistent.

The same pattern exists for release (lines 106-109):

c.SDL_EVENT_KEY_UP => {
    const key = Key.fromSDL(event.key.key);
    _ = self.keys_down.remove(key);                   // Line 108 - succeeds
    self.keys_released.put(key, {}) catch {}; // Line 109 - FAILS, key not in keys_released
},

🛠️ Proposed Fix

When keys_pressed.put() fails, remove the key from keys_down to maintain consistency:

c.SDL_EVENT_KEY_DOWN => {
    if (!event.key.repeat) {
        const key = Key.fromSDL(event.key.key);
        self.keys_down.put(key, {}) catch {};
        if (self.keys_pressed.put(key, {}) catch {}) |_| {
            _ = self.keys_down.remove(key);  // Roll back keys_down on keys_pressed failure
        }
    }
},

Alternatively, propagate the error and let the caller decide whether to crash or roll back.

✅ Acceptance Criteria

• isKeyDown and isKeyPressed return consistent results for the same key (both true when key is both held AND was pressed this frame)
• Unit test verifies state consistency when allocation fails mid-operation
• No keys become "stuck" in keys_down after failed press registration
• zig build test passes

📚 References

• Related: Issue [Audit][High] Mouse wheel scroll values are overwritten instead of accumulated per frame #711 (scroll accumulation - same catch {} error handling pattern)
• Related: Issue [Audit][Medium] InputBinding.eql() treats .key and .key_alt as equivalent, breaking distinct binding behavior #729 (InputBinding.eql - related input state correctness)
• Zig std.mem.Allocator error handling patterns

[github-actions]

The issue is validated. Lines 102-103 and 108-109 have the exact inconsistency described. When keys_pressed.put() fails on line 103, keys_down already has the key from line 102, but keys_pressed does not — creating the inconsistent state.

The proposed fix of rolling back keys_down on keys_pressed failure is a reasonable approach. The same rollback logic applies to the release path (line 109) where keys_down.remove() succeeds before keys_released.put() could fail.

Bug Classification: Confirmed bug — state inconsistency from silent error handling (catch {}).

Suggested implementation: The rollback approach is correct but could be refactored to avoid duplication:

c.SDL_EVENT_KEY_DOWN => {
    if (!event.key.repeat) {
        const key = Key.fromSDL(event.key.key);
        self.keys_down.put(key, {}) catch {};
        if (self.keys_pressed.put(key, {}) catch |_| {
            _ = self.keys_down.remove(key);
        });
    }
},

This is a valid, self-contained fix. No need to break into sub-issues — single clear fix in one file.

opencode session  |  github run