From 6f0f95f4fb29cfacb8602039b0b1b76bc2fc1999 Mon Sep 17 00:00:00 2001 From: DanS Date: Wed, 15 Jul 2026 11:55:11 -0500 Subject: [PATCH] fix(image-picker): avoid shutdown UB in the async decode counter Adversarial review of the off-thread decode found the one real defect: a detached decode worker decremented the static s_animInFlight counter, which at process exit could run after static destruction begins (shutdown UB). Make the counter a heap std::shared_ptr the worker co-owns, so it safely outlives teardown; the worker now touches only heap objects it holds a share of. Also noted s_animatingThisFrame is intentionally main-thread-only. (Review confirmed the rest: worker never touches the thumb map, done release/acquire publishes the frames, GL upload stays on the UI thread, stb is thread_local + libwebp per-instance so concurrent decodes are safe, the in-flight slot is never leaked, and growth is bounded/cleared on navigate.) Co-Authored-By: Claude Opus 4.8 (1M context) --- src/ui/windows/image_picker.h | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/ui/windows/image_picker.h b/src/ui/windows/image_picker.h index 503b815..407d6c1 100644 --- a/src/ui/windows/image_picker.h +++ b/src/ui/windows/image_picker.h @@ -357,16 +357,17 @@ private: static void driveThumbAnim(Thumb& th, const std::string& path) { if (th.animReady) return; if (!th.animRequested) { - if (s_animInFlight.load() >= kMaxAnimInFlight) return; // cap workers; retry a later frame + if (s_animInFlight->load() >= kMaxAnimInFlight) return; // cap workers; retry a later frame th.animRequested = true; th.job = std::make_shared(); - s_animInFlight.fetch_add(1); + s_animInFlight->fetch_add(1); std::shared_ptr job = th.job; + std::shared_ptr> inflight = s_animInFlight; // co-owned: safe past teardown std::string p = path; - std::thread([job, p]() { + std::thread([job, inflight, p]() { util::LoadAnimatedRGBA(p.c_str(), kThumbMaxFrames, job->result); job->done.store(true, std::memory_order_release); - s_animInFlight.fetch_sub(1); + inflight->fetch_sub(1); }).detach(); return; } @@ -510,8 +511,12 @@ private: static constexpr int kThumbMaxFrames = 300; // cap frames per hovered animation static constexpr int kMaxAnimInFlight = 2; // max concurrent background decodes - static inline std::atomic s_animInFlight{0}; + // Heap-owned so a detached worker (which decrements it) can safely outlive static teardown at + // process exit — the worker holds a shared_ptr copy, so it never touches a destroyed static. + static inline std::shared_ptr> s_animInFlight = std::make_shared>(0); static inline bool s_open = false; + // Main-thread-only (set in render/driveThumbAnim, read+cleared in consumeAnimationActive); NOT atomic + // on purpose — do not read/write it from a worker thread. static inline bool s_animatingThisFrame = false; // a hover-preview animation is playing static inline int s_loadedThisFrame = 0; static inline std::string s_dir;