[Audit][High] Double-cleanup risk: JobQueue.doReprioritize drops jobs on OOM but leaves cleanup_fn intact, causing double-free on queue shutdown

[MichaelFisher1997]

🔍 Module Scanned\n (automated audit scan)\n\n## 📝 Summary\nIn , when re-adding jobs after reprioritization fails with an OOM error, the job's is not nullified after being called. This means if the job is later re-added to the queue and the queue is eventually stopped/cleared, the will be called a second time — potentially causing a double-free if the cleanup function deallocates memory.\n\n## 📍 Location\n- File: \n- Function/Scope: \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\nIf a job context involves heap-allocated memory (common for generic jobs with custom context), a double-cleanup can cause:\n- Double-free: Application crash with heap corruption\n- Use-after-free: Silent memory corruption that may manifest later\n- Resource leak: If cleanup_fn sets itself to null after freeing, second call becomes a no-op but the first catch block already leaked the job's chance to be re-added properly\n\n## 🔎 Evidence\n\n\nThe block at line 224 calls on OOM, but doesn't nullify . If this job is later successfully pushed (line 233), it retains its . When or eventually pops and cleans this job again, double-cleanup occurs.\n\n## 🛠️ Proposed Fix\nAfter calling on a job that should not be re-added to the queue, nullify its to prevent double-cleanup:\n\n\n\nAlternatively, introduce a helper on that clears the cleanup function pointer after cleanup is invoked.\n\n## ✅ Acceptance Criteria\n- [ ] No double-cleanup occurs when OOM path is triggered\n- [ ] All unit tests in pass\n- [ ] The fix has been verified with Zig 0.16.0 + SDL3 Dev Environment

Compiler: 0.16.0
assets/shaders/vulkan/water.vert
assets/shaders/vulkan/bloom_downsample.vert
assets/shaders/vulkan/bloom_upsample.vert
assets/shaders/vulkan/debug_shadow.vert
assets/shaders/vulkan/fxaa.vert
assets/shaders/vulkan/post_process.vert
assets/shaders/vulkan/shadow.vert
assets/shaders/vulkan/sky.vert
assets/shaders/vulkan/ssao.vert
assets/shaders/vulkan/taa.vert
assets/shaders/vulkan/terrain.vert
assets/shaders/vulkan/ui.vert
assets/shaders/vulkan/ui_tex.vert
assets/shaders/vulkan/water.vert
assets/shaders/vulkan/bloom_downsample.frag
assets/shaders/vulkan/bloom_upsample.frag
assets/shaders/vulkan/debug_shadow.frag
assets/shaders/vulkan/fxaa.frag
assets/shaders/vulkan/g_pass.frag
assets/shaders/vulkan/post_process.frag
assets/shaders/vulkan/shadow.frag
assets/shaders/vulkan/sky.frag
assets/shaders/vulkan/ssao.frag
assets/shaders/vulkan/ssao_blur.frag
assets/shaders/vulkan/taa.frag
assets/shaders/vulkan/terrain.frag
assets/shaders/vulkan/terrain_debug.frag
assets/shaders/vulkan/ui.frag
assets/shaders/vulkan/ui_tex.frag
assets/shaders/vulkan/water.frag
assets/shaders/vulkan/culling.comp
assets/shaders/vulkan/depth_pyramid.comp
assets/shaders/vulkan/lpv_inject.comp
assets/shaders/vulkan/lpv_propagate.comp
assets/shaders/vulkan/mesh.comp
assets/shaders/vulkan/shadow.vert
assets/shaders/vulkan/mesh.comp
assets/shaders/vulkan/debug_shadow.vert
assets/shaders/vulkan/g_pass.frag
assets/shaders/vulkan/terrain.frag
assets/shaders/vulkan/ui_tex.vert
assets/shaders/vulkan/sky.frag
assets/shaders/vulkan/sky.vert
assets/shaders/vulkan/depth_pyramid.comp
assets/shaders/vulkan/lpv_propagate.comp
assets/shaders/vulkan/ssao.vert
assets/shaders/vulkan/culling.comp
assets/shaders/vulkan/debug_shadow.frag
assets/shaders/vulkan/terrain.vert
assets/shaders/vulkan/water.frag
assets/shaders/vulkan/ssao_blur.frag
assets/shaders/vulkan/ui.frag
assets/shaders/vulkan/taa.vert
assets/shaders/vulkan/taa.frag
assets/shaders/vulkan/ssao.frag
assets/shaders/vulkan/ui.vert
assets/shaders/vulkan/ui_tex.frag
assets/shaders/vulkan/shadow.frag
assets/shaders/vulkan/lpv_inject.comp\n- [ ] Generic jobs with heap-allocated context survive without double-free\n\n## 📚 References\n- Zig errdefer pattern: https://ziglang.org/documentation/master/#Error-Recovery\n- Prior art: and both call and rely on jobs not being re-added after cleanup\n- Related: Issue #722 (TreeHintCache.put silently swallows OOM) — similar OOM handling pattern\n

[github-actions]

Valid bug. Looking at job_system.zig:224-228 and job_system.zig:233-237, when OOM causes job cleanup, cleanup_fn is not nullified. If the job is later re-added and the queue is later cleared/stopped, cleanup() runs a second time causing a double-free.

The Job struct has no method to nullify cleanup_fn — it can only be accessed via Job.data.generic.cleanup_fn directly.

Comment:

This is a valid bug. The double-cleanup risk is real at two locations in doReprioritize:

1. Line 226: updated_job.cleanup() on OOM during temp.append — cleanup_fn not cleared
2. Line 235: job.cleanup() on OOM during jobs.push — cleanup_fn not cleared

The Job.cleanup() method at line 93-102 calls cleanup_fn but never nullifies it. The GenericJobData struct at line 87-91 has cleanup_fn as a nullable pointer that is never reset after invocation.

Proposed fix path:

1. Either add a Job.clearCleanupFn() method that nullifies cleanup_fn after cleanup, or nullify it inline at the call sites
2. The fix at lines 226 and 235 should be: job.data.generic.cleanup_fn = null; job.cleanup(); (or call a helper that does both)

This is self-contained to modules/engine-core/src/job_system.zig and doesn't span multiple subsystems.

opencode session  |  github run