fix: deadlock in get_system_info_with_cache() during recipe computation (#2414) - #2453
Conversation
…r OpenAI compat Closes lemonade-sdk#1370 — OpenCode / @ai-sdk/openai-compatible streaming crash Three fixes for reasoning model streaming: 1. Streaming proxy normalization (streaming_proxy.cpp): - Intercepts each SSE data: {...} line in forward_sse_stream() - Injects content: "" when reasoning_content is present without content - Injects role: "assistant" when null/missing on assistant deltas - Only applies to chat.completion.chunk objects (non-chat passthrough) 2. Non-streaming response (server.cpp): - Same content injection for REST chat completions response 3. thinking: false passthrough (server.cpp): - Replaced strip_handled_thinking_fields() (which erased enable_thinking/ thinking before forwarding) with normalize_thinking_fields() which renames thinking → enable_thinking and keeps it in the forwarded request. FLM/vLLM/cloud backends now see enable_thinking. - /no_think prefix retained for llama.cpp compatibility Tests: 11 unit tests covering role normalization, reasoning content normalization, carriage return, multi-choice, multi-line streams. 7/7 C++ tests pass (100%).
Closes lemonade-sdk#2371 — multi-GPU systems only got ROCm for the first GPU arch get_rocm_arch() iterates AMD GPUs (iGPU first, then dGPU) and returns only the first match. On systems with both an iGPU and dGPU with different architectures, TheRock was only installed for the iGPU. Fix: - Add get_rocm_arches() returning ALL detected AMD GPU architectures (deduplicated, iGPU-first ordering preserved) - Update install_therock_if_needed() to install TheRock for every arch - Keep get_rocm_arch() for backward compat (rocm_channel, display)
…dk#1364, lemonade-sdk#1546) lemonade-sdk#1364 — Large Prompts Timing Out - Add SSE keepalive heartbeat thread in forward_sse_stream() - Sends : keepalive\n\n every 10s during prefill while waiting for first token - Prevents client-side read timeouts on long-running prompt processing - Thread-safe via shared mutex with the libcurl write callback lemonade-sdk#1546 — Model Download Resilience - Add .completed sentinel written after all files are verified in download - is_checkpoint_path_complete() checks for .completed as authoritative marker - Prevents corrupt partially-downloaded files from appearing complete - Hardened recursive_directory_iterator with skip_permission_denied + error_code
Add a pre-load memory check in Router::load_model() that compares model_info.size (file size in GB) against get_available_memory_gb() for the target device. Logs a warning when the model may not fit. Chose warn-only (not block) because: 1. GGUF file size != load-time memory (mmap'd, paged) 2. Auto-tune ctx_size resolver will reduce context to fit 3. A hard block would frustrate users who know their setup
Completes fixes for lemonade-sdk#1364, lemonade-sdk#1546, lemonade-sdk#1804: ## lemonade-sdk#1364 — SSE heartbeat during long prefill Injects : keepalive\n\n every 10s during prefill to prevent client-side read timeouts on long prompts (15k tokens → 5 min prefill). ## #1546b — .completed sentinel for download verification Written after all files are verified in download_from_huggingface(). is_checkpoint_path_complete() checks for it, preventing corrupt partially- downloaded files from appearing complete after a crash. ## #1546a — Model-level download resume fast-path download_from_huggingface() now accepts do_not_upgrade flag. When set and .completed sentinel exists, skips the HF API call entirely. ## #1546c — Directory iterator hardening discover_extra_models() uses skip_permission_denied + error_code handling to prevent crashes on temp files from interrupted downloads. ## lemonade-sdk#1804 — Pre-load OOM guard Upgraded the pre-load memory check from warning to hard block when model size exceeds 2x available memory headroom. Prevents OOM killer crashes with a clear error message instead. ## CI — PR-Agent + Qodo dual review Added pr-agent-review.yml (DeepSeek) and qodo-merge.yml workflows.
Before fetching the HF API and rebuilding the file list, check for an existing .download_manifest.json with incomplete files. If found, resume downloading from the partial state instead of starting over. This avoids re-downloading already-completed files after a network interruption or Ctrl-C during model pull.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb60d642f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| std::lock_guard<std::mutex> lock(s_system_info_mutex); | ||
| s_cached_system_info = system_info; |
There was a problem hiding this comment.
When two HTTP workers miss s_hardware_computed at startup, both run hardware detection outside the mutex. If the slower worker reaches this store after the faster worker has already added recipes and set s_recipes_computed=true, this assignment replaces the whole cached JSON with hardware-only data while the recipe flag stays true; future fast-path calls then permanently return a cache with no recipes, so model support checks can report recipes missing until invalidation or restart. Recheck under the mutex before overwriting, or merge the hardware fields without discarding existing recipes.
Useful? React with 👍 / 👎.
| if (!s_computing_recipes.compare_exchange_strong(expected, true, | ||
| std::memory_order_acq_rel)) { | ||
| // Re-entrant call during recipe computation — return hardware info | ||
| std::lock_guard<std::mutex> lock(s_system_info_mutex); | ||
| return s_cached_system_info; |
There was a problem hiding this comment.
When a non-recursive request arrives while another worker is computing recipes, this global compare_exchange also fails and returns s_cached_system_info without recipes. Callers such as SystemInfo::get_supported_backends() and SystemInfoCache::get_flm_status() treat missing recipes as unsupported/not found, so concurrent /models, load, or install-status requests during startup or recipe invalidation can fail spuriously; only the same-thread reentrant get_rocm_arch()/get_cuda_arch() path needs hardware-only data, while other threads should wait for recipe computation to finish.
Useful? React with 👍 / 👎.
| } | ||
| s_recipes_computed = true; | ||
|
|
||
| s_recipes_computed.store(true, std::memory_order_release); |
There was a problem hiding this comment.
If an install/uninstall, config change, or SIGHUP calls SystemInfoCache::invalidate_recipes() while another request is already inside build_recipes_info(), that invalidation can be overwritten here: the invalidating thread sets s_recipes_computed=false, then this older computation stores stale recipes and flips the flag back to true. Before this change the mutex forced invalidation to run after the in-flight computation, so the next request recomputed; now backend status can remain stale after a completed install or config change until a later invalidation/restart.
Useful? React with 👍 / 👎.
This reverts commit 11991bc.
…ection (lemonade-sdk#2414) The function used a std::lock_guard which caused a deadlock when build_recipes_info re-entered get_system_info_with_cache via get_rocm_arch(). Fix by switching to std::unique_lock, marking s_recipes_computed early, and unlocking during recipe computation. On failure the flag resets so the next call retries.
bb60d64 to
b96224c
Compare
1 participant
Summary
Fixes a re-entrant deadlock in `SystemInfoCache::get_system_info_with_cache()` that prevents `lemond` from starting on systems with AMD GPUs (bisected to commit 37c1a56 / #2295).
Root Cause
`get_system_info_with_cache()` held a `std::lock_guardstd::mutex` across BOTH hardware detection AND recipe computation. During recipe computation, the chain:
```
SystemInfoCache::get_system_info_with_cache() [lock acquired]
└─ build_recipes_info()
└─ bm->get_or_resolve_latest_tag()
└─ install_params_fn()
└─ SystemInfo::get_rocm_arch()
└─ SystemInfoCache::get_system_info_with_cache() [DEADLOCK ✗]
```
`get_rocm_arch()` calls `get_system_info_with_cache()` to read detected AMD GPUs from the cache, but that function already holds the non-recursive mutex on the same thread.
Fix
Two changes to `system_info.cpp`:
Narrow lock scopes — compute hardware and recipes OUTSIDE the mutex; lock only to read/write the cached JSON.
Atomic re-entrant guard — a `std::atomic s_computing_recipes` flag prevents infinite recursion: if `get_system_info_with_cache()` is called re-entrantly during recipe computation, it returns the partial cache (hardware info without recipes), which is sufficient for `get_rocm_arch()` / `get_cuda_arch()`.
Testing