[Audit][High] ImGui context leaked on init failure in imgui_backend.zig

[MichaelFisher1997]

Summary

The Backend.init function in imgui_backend.zig creates an ImGui context via ZigCraft_ImGui_CreateContext() at line 21, but if any subsequent initialization step fails (SDL3 backend at line 24 or Vulkan backend at line 44), the context is never destroyed. This causes a memory leak each time ImGui initialization fails.

Location

• File: modules/engine-ui/src/imgui/imgui_backend.zig:21-47
• Function/Scope: Backend.init

Severity: High

Memory leak on ImGui initialization failure

Impact

Every time the ImGui backend fails to initialize (e.g., due to Vulkan device loss, SDL3 window handle issues, or driver problems), the ImGui context allocated by ZigCraft_ImGui_CreateContext() is never freed. This leak accumulates over repeated initialization attempts and can cause memory exhaustion in long-running applications or when the graphics context is reset.

Evidence

In the init function, the ImGui context is created at line 21:

c.ZigCraft_ImGui_CreateContext();  // Line 21 - Context CREATED here

But if ZigCraft_ImGui_ImplSDL3_InitForVulkan fails at line 24, the function returns without destroying the context:

if (!c.ZigCraft_ImGui_ImplSDL3_InitForVulkan(@ptrCast(window))) {
    log.log.err("Failed to initialize ImGui SDL3 backend", .{});
    return error.ImguiBackendInitFailed;  // Line 26 - BUG: context NOT destroyed!
}

Similarly, if ZigCraft_ImGui_ImplVulkan_Init fails at line 44, only SDL3 is shut down but the context is not destroyed:

if (!c.ZigCraft_ImGui_ImplVulkan_Init(&init_info)) {
    c.ZigCraft_ImGui_ImplSDL3_Shutdown();  // Line 45 - SDL3 shut down
    log.log.err("Failed to initialize ImGui Vulkan backend", .{});
    return error.ImguiBackendInitFailed;  // Line 47 - BUG: context NOT destroyed!
}

Compare with deinit() which properly destroys the context:

pub fn deinit(self: *Backend) void {
    if (!build_options.imgui or !self.initialized) return;
    c.ZigCraft_ImGui_ImplVulkan_Shutdown();
    c.ZigCraft_ImGui_ImplSDL3_Shutdown();
    c.ZigCraft_ImGui_DestroyContext();  // Context properly destroyed here
    self.initialized = false;
}

The issue is that deinit() is never called when init() returns early with an error. The context created at line 21 becomes orphaned.

Proposed Fix

Add error recovery using errdefer to destroy the context on any failure path:

pub fn init(window: *sdl.SDL_Window, rhi_ptr: *rhi.RHI) !Backend {
    if (!build_options.imgui) return error.ImguiDisabled;

    errdefer c.ZigCraft_ImGui_DestroyContext();

    c.ZigCraft_ImGui_CreateContext();
    errdefer {
        c.ZigCraft_ImGui_ImplSDL3_Shutdown();
        c.ZigCraft_ImGui_DestroyContext();
    }
    c.ZigCraft_ImGui_StyleColorsDark();

    if (!c.ZigCraft_ImGui_ImplSDL3_InitForVulkan(@ptrCast(window))) {
        log.log.err("Failed to initialize ImGui SDL3 backend", .{});
        return error.ImguiBackendInitFailed;
    }

    const native = rhi_ptr.nativeHandles();
    // ... setup init_info ...

    if (!c.ZigCraft_ImGui_ImplVulkan_Init(&init_info)) {
        c.ZigCraft_ImGui_ImplVulkan_Shutdown();
        c.ZigCraft_ImGui_ImplSDL3_Shutdown();
        log.log.err("Failed to initialize ImGui Vulkan backend", .{});
        return error.ImguiBackendInitFailed;
    }

    return .{ .initialized = true };
}

Acceptance Criteria

• nix develop --command zig build test passes with no regressions
• No memory leak when ImGui init fails (verified by code inspection that ZigCraft_ImGui_CreateContext has a corresponding ZigCraft_ImGui_DestroyContext on all code paths)
• ImGui backend still functions correctly when init succeeds
• The fix has been verified with nix develop --command zig build test

References

• Dear ImGui CreateContext() / DestroyContext() documentation
• Zig errdefer pattern for guaranteed cleanup: https://ziglang.org/documentation/master/#errdefer

[github-actions]

The issue is validated. The bug is real and well-documented.

Analysis:

• Line 21: ZigCraft_ImGui_CreateContext() creates the context
• Lines 24-26: If SDL3 init fails, the context is orphaned (no DestroyContext call)
• Lines 44-47: If Vulkan init fails, only ZigCraft_ImGui_ImplSDL3_Shutdown() is called at line 45, but ZigCraft_ImGui_DestroyContext() is never invoked
• Lines 53-59: deinit() correctly destroys the context, but is never called on init() failure paths

Proposed Fix Validation:
The errdefer approach is correct Zig idiom. The fix properly handles both failure paths:

1. SDL3 init failure: errdefer destroys context directly
2. Vulkan init failure: errdefer block runs SDL3_Shutdown() then DestroyContext()

Classification: Bug (confirmed memory leak)

Scope: Simple, localized fix - no need to break into sub-issues.

Recommendation: The proposed fix is correct. Implementation should use errdefer as shown, with the inner errdefer block in the SDL3 init block ensuring both SDL3 shutdown and context destruction on Vulkan init failure.

opencode session  |  github run