[Audit][High] JobQueue.doReprioritize leaks jobs on re-add failure

[MichaelFisher1997]

🔍 Module Scanned

src/engine/core/ (automated audit scan)

📝 Summary

In JobQueue.doReprioritize, when re-adding jobs to the priority queue fails (due to an allocation error), the job is silently dropped without calling its cleanup_fn. This causes a memory leak for generic jobs that own heap-allocated context.

📍 Location

• File: src/engine/core/job_system.zig:226-229
• Function/Scope: JobQueue.doReprioritize

🔴 Severity: High

• High: Memory leaks, race conditions, incorrect rendering, broken features

💥 Impact

When the priority queue's underlying array grows during add() and the allocation fails, any generic job that was successfully extracted from the original queue and appended to the temporary list is permanently lost. The cleanup_fn is never called, leaking any heap-allocated context the job owns. This accumulates over time as the player moves and the queue is repeatedly reprioritized.

🔎 Evidence

// Re-add with updated priorities
for (temp.items) |job| {
    self.jobs.add(job) catch {
        log.log.warn("Job queue: failed to re-add job after priority update", .{});
        continue;  // <-- BUG: job dropped, cleanup never called
    };
}

Compare to the correct pattern in doReprioritize at line 217-221:

temp.append(self.allocator, updated_job) catch {
    log.log.warn("Job queue: dropped job during priority update (allocation failed)", .{});
    updated_job.cleanup();  // <-- CORRECT: cleanup is called
    continue;
};

The first catch block (line 219) correctly calls updated_job.cleanup() before dropping the job. The second catch block (line 226-229) does not, causing a leak.

🛠️ Proposed Fix

Add job.cleanup() before continue in the second catch block at line 226:

self.jobs.add(job) catch {
    log.log.warn("Job queue: failed to re-add job after priority update", .{});
    job.cleanup();  // Release generic job context before dropping
    continue;
};

✅ Acceptance Criteria

• All unit tests in src/engine/core/job_system.zig pass
• Generic jobs with cleanup_fn that fail to re-prioritize have their cleanup called
• No memory leaks when std.mem.Allocator fails during jobs.add() in doReprioritize

📚 References

• Related: Job.cleanup() at line 92-101 which handles cleanup for generic jobs
• Related: doReprioritize at line 217-221 which correctly calls cleanup before dropping jobs

[github-actions]

The issue is valid and accurately describes a memory leak bug.

Classification: Bug (High severity - memory leak)

Verification: I reviewed the code at src/engine/core/job_system.zig:

• Lines 217-221: temp.append(...) catch { ... updated_job.cleanup(); continue; } — correctly calls cleanup
• Lines 226-229: self.jobs.add(job) catch { ... continue; } — missing job.cleanup() before continue

Impact: When self.jobs.add(job) fails (OOM or arena exhaustion), any generic Job with a cleanup_fn is silently dropped without releasing its heap-allocated context. This accumulates over time as the player moves and doReprioritize is called repeatedly.

Proposed Fix: Correct. The fix at line 227-228 should be:

self.jobs.add(job) catch {
    log.log.warn("Job queue: failed to re-add job after priority update", .{});
    job.cleanup();
    continue;
};

Recommendation: The fix is trivial and self-contained. The existing unit tests in src/engine/core/job_system.zig should cover this path. No breaking changes or refactoring required.

opencode session  |  github run

[MichaelFisher1997]

/oc investigate this PR and comments.
if its still valid, solve it and submit it as a PR against dev

[MichaelFisher1997]

PR created: #493

Fix applied: Added job.cleanup() before continue at line 228 in job_system.zig:227 to ensure generic jobs have their cleanup called when re-adding to the priority queue fails.

opencode session  |  github run