DragonX/ObsidianDragon
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 4 Wiki Activity

fix(image-picker): avoid shutdown UB in the async decode counter

Browse Source 
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<atomic> 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) <noreply@anthropic.com>
This commit is contained in:
DanS
2026-07-15 11:55:11 -05:00
parent a7c50ff19b
commit 6f0f95f4fb
1 changed files with 10 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
src/ui/windows/image_picker.h
View File

@@ -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<AnimJob>();
s_animInFlight.fetch_add(1);
s_animInFlight->fetch_add(1);
std::shared_ptr<AnimJob> job = th.job;
std::shared_ptr<std::atomic<int>> 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<int> 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<std::atomic<int>> s_animInFlight = std::make_shared<std::atomic<int>>(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;