fix the deadlock at server startup

[ckuethe]

Symptom: lemond starts, prints something about "Building models cache", prints some other stuff, but never says it "Cache built". A handful of debug prints show that there's a path calling get_system_info_with_cache that recursively calls get_system_info_with_cache ... which tries to get a mutex on s_system_info_mutex. "Well there's your problem!"

I'm also not sure why I'm only just running into this now, but it fixes #2414

Co-authored-by: opencode, Qwen3.6-35B-A3B-MTP

[ckuethe]

@claude review

[bong-water-water-bong]

In-Depth Review — Found Critical Bug + Several Hardening Issues

Thanks for this fix! The overall approach (overload pattern + lock scope reduction + recursion guard + sysfs bypass) is sound. However, I found several issues during a thorough review, including one critical bug that prevents recipe caching from ever working.

🔴 Critical: is_outermost_call always false — recipes never cached

In get_system_info_with_cache(), the original code determines the outermost caller with:

bool is_outermost_call = !s_phase2_in_progress.exchange(false);

exchange(false) returns the old value. After the earlier exchange(true) set it to true, this returns true, so !true = false. The outermost caller never writes recipes to cache, s_recipes_computed stays false forever, and Phase 2 recomputes on every single call.

Fix: The variable was_already_in_progress already captures the information needed — it is false iff this thread set the flag from false→true. Use that directly. Also restructured to hoist the declaration outside the try block so it is in scope after the catch, and only the outermost caller resets the flag.

🟠 Moderate: detect_rocm_arch_from_sysfs() Priority 2 is dead code

The PCI device ID → gfx target mapping loop reads vendor and device IDs from sysfs, but the mapping table between device IDs and gfx architecture strings was entirely missing — just a comment where the code should have been. On systems without /sys/class/kfd/kfd/topology/nodes, the fallback silently returns "".

Fix: Added the full mapping table covering RDNA2 (Navi 21/22/23/24), RDNA3 (Navi 31/32/33), RDNA3.5 iGPU (Strix Point, Strix Halo, Krackan Point), and RDNA4 (Navi 48).

Additional Hardening (6 more fixes)

Issue	Fix
KFD std::stoi failure returns "" immediately instead of falling through to Priority 2	Changed to continue — tries next KFD node, then falls through to PCI mapping
return s_cached_system_info without lock — data race with concurrent Phase 2 writer installing ["recipes"]	Wrapped return in lock_guard<s_system_info_mutex> to copy under lock
invalidate_recipes() races with in-progress Phase 2 — invalidation clears s_recipes_computed, but Phase 2 finishes and sets it back to true, caching stale recipes	Added std::atomic<uint64_t> s_recipe_generation counter; Phase 2 snapshots before computing and re-checks under lock
Failed computation caches empty recipes — if create_system_info() or build_recipes_info() throws, empty recipes were cached and s_recipes_computed set to true, preventing retry	Added computation_ok flag; failures leave s_recipes_computed = false
Stale lock-ordering comments in headers and cpp referencing old models_cache_mutex_ → s_system_info_mutex ordering	Updated to describe current ordering
PCI_GFX_MAP key case mismatch — sysfs outputs lowercase hex	Map keys are now lowercase

Lock Ordering Verified

Checked all 30+ models_cache_mutex_ acquisition sites — zero violations of s_system_info_mutex → models_cache_mutex_.

Edge Cases Traced

Single-threaded startup, two-thread race into build_cache(), recursive call during Phase 2, concurrent invalidate_recipes() during computation, exception during computation, nested caller returning while outermost writes, unparseable KFD data, system without KFD topology nodes, system without AMD GPU — all handled correctly.

Fixes available at

bong-water-water-bong/1bit-lemonade:fix/deadlock-review-fixes (commit f230620a)

[ckuethe]

thanks. I'll have a look at that tomorrow

[bong-water-water-bong]

@ckuethe I completed an in-depth review of this PR and found several issues including one critical bug where is_outermost_call is always false, preventing recipe caching from ever working (Phase 2 recomputes on every call). I also found and fixed a missing PCI device ID→gfx target mapping table in detect_rocm_arch_from_sysfs(), several data races in CloudServer, exception safety holes in the state machine, and a cache corruption bug in the cloud uninstall path.

All fixes are on bong-water-water-bong:pr-2443 (2 commits on top of yours):

git pull https://github.com/bong-water-water-bong/1bit-lemonade.git pr-2443

The branch compiles cleanly (lemond + lemonade CLI, zero warnings). Full details in the review above. Let me know if you have any questions.

🤖 Generated with Claude Code

[bong-water-water-bong]

@ckuethe Updated the fix branch with additional hardening from a full 4-agent review. Here is the complete summary:

What was found and fixed (14 issues)

Critical

• is_outermost_call always false — recipes never cached, s_recipes_computed stuck false forever. Replaced with correct was_already_in_progress check.
• PCI device ID → gfx target mapping missing in detect_rocm_arch_from_sysfs() — added RDNA2–RDNA4 table

High

• restore() exception leaves state machine hung — added try/catch with DOWNSIZED recovery
• downsize() throws → maintenance_in_progress_ stuck true forever — wrapped in try/catch in eviction engine
• Data race on CloudServer::loaded_ (plain bool) — made std::atomic<bool> with acquire/release semantics
• Data race on CloudServer::upstream_model_ — protected by acquire/release via loaded_

Medium

• evict_cloud_models() called before verifying uninstall() succeeded — moved after 404 check
• ~Server() destructor can throw via cancel_download_jobs() — wrapped in try/catch
• localtime_r with UTC Z format — switched to gmtime_r
• target_tokens * 4 overflow on untrusted input — capped
• write_json_file() never checked write success — added out.good() check

Hardening

• Generation counter prevents invalidate_recipes() from racing with in-progress Phase 2
• computation_ok flag prevents caching failed recipe computation
• Return value of get_system_info_with_cache() now copied under lock
• KFD std::stoi failure now falls through to Priority 2 instead of aborting

Test results

• lemond + lemonade build: zero warnings, zero errors
• Server startup: no hang, cache built in ~750ms
• CUDA Arch Mapping: 10/10 pass
• Device Family Matching: 5/5 pass
• System Info endpoint returns correct recipes

Pull

git pull https://github.com/bong-water-water-bong/1bit-lemonade.git pr-2443

🤖 Generated with Claude Code

[bong-water-water-bong]

@ckuethe GitHub CLI does not support cross-fork PR creation, but you can create the PR with one click here:

ckuethe/lemonade@startup_deadlock...bong-water-water-bong:pr-2443

That link opens a PR from bong-water-water-bong:pr-2443 into your startup_deadlock branch with the full diff pre-populated.

[bong-water-water-bong]

Found a critical bug in the is_outermost_call logic:

system_info.cpp: bool is_outermost_call = !s_phase2_in_progress.exchange(false);

exchange() returns the PREVIOUS value. For the outermost caller, the previous value is true (set by the earlier exchange(true)), so !true = false. This means is_outermost_call is always false — recipes are never written to cache, and s_recipes_computed is never set to true. This causes:

• FLM models never discovered (get_flm_status returns unsupported)
• All recipe checks fail (no recipes in system_info)
• filter_models_by_backend filters out all models
• Phase 2 recomputes on every single invocation (redundant I/O)

Fix (one line): The existing was_already_in_progress variable already correctly identifies the outermost caller. Replace:

bool is_outermost_call = !s_phase2_in_progress.exchange(false);

with:

bool is_outermost_call = !was_already_in_progress;
s_phase2_in_progress.store(false);

I've opened PR #2449 with this fix: #2449

[ckuethe]

pulled your bong-water-water-bong@a7fb81e into this PR

[ckuethe]

Thanks for the feedback @bong-water-water-bong

[jeremyfowers]

Let's evaluate #2488 as a potentially simpler alternative.

[ckuethe]

#2488 works too, and is a smaller, simpler diff than this one.