server: fix deadlock in load_models() when erasing a finished download (#25358)
* server: fix deadlock in load_models() when erasing a finished download The download monitoring thread acquires the models mutex on its way out, but load_models() joined it from the erase loop while holding that mutex. Join it outside the lock via threads_to_join like the other monitoring threads. * server: add default timeout to test requests A hung server now fails the test after 10 minutes instead of stalling the CI job for hours. Explicit timeouts are unchanged.
This commit is contained in:
@@ -523,6 +523,7 @@ void server_models::load_models() {
|
||||
|
||||
// collect all threads to join in one pass while the lock is held:
|
||||
// - monitoring threads from just-unloaded models (to_unload)
|
||||
// - threads of finished downloads (DOWNLOADED), they acquire the mutex on exit
|
||||
// - threads of already-UNLOADED models that are being removed from source
|
||||
std::vector<std::thread> threads_to_join;
|
||||
for (const auto & name : to_unload) {
|
||||
@@ -535,6 +536,13 @@ void server_models::load_models() {
|
||||
if (inst.meta.status == SERVER_MODEL_STATUS_DOWNLOADING) {
|
||||
continue; // downloading models are not from config sources, leave them alone
|
||||
}
|
||||
if (inst.meta.status == SERVER_MODEL_STATUS_DOWNLOADED) {
|
||||
// joining this thread under the lock deadlocks: it locks the mutex on its way out
|
||||
if (inst.th.joinable()) {
|
||||
threads_to_join.push_back(std::move(inst.th));
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (final_presets.find(name) == final_presets.end() && !inst.meta.is_running() && inst.th.joinable()) {
|
||||
threads_to_join.push_back(std::move(inst.th));
|
||||
}
|
||||
@@ -550,10 +558,8 @@ void server_models::load_models() {
|
||||
if (it->second.meta.status == SERVER_MODEL_STATUS_DOWNLOADING) {
|
||||
++it; // download thread is still busy, skip
|
||||
} else if (it->second.meta.status == SERVER_MODEL_STATUS_DOWNLOADED) {
|
||||
// download finished, safe to erase
|
||||
if (it->second.th.joinable()) {
|
||||
it->second.th.join();
|
||||
}
|
||||
// download finished, thread is joined above, safe to erase
|
||||
GGML_ASSERT(!it->second.th.joinable());
|
||||
it = mapping.erase(it);
|
||||
} else if (final_presets.find(it->first) == final_presets.end()) {
|
||||
SRV_INF("(reload) removing model name=%s (no longer in source)\n", it->first.c_str());
|
||||
|
||||
Reference in New Issue
Block a user