From 991991118571bf9e2c8bfd46593b230e35fe5796 Mon Sep 17 00:00:00 2001 From: Pascal Date: Tue, 22 Sep 2026 21:38:54 +0200 Subject: [PATCH] server: fix router eviction races with the existing queue (#29217) * server: route every model load through the queue A model loaded by the fast path has no queue entry, so tick() evicts it at its LOADED transition before its own request is proxied. Every load now joins the queue, whose entry protects the model until its waiters leave. * server: do not admit requests into a stopping model A request for a model that is being stopped still sees it LOADED and is proxied into the dying child. Such a request now joins the queue and is served by the next instance. The stopping mark is cleared under the same lock that sets UNLOADED, so no request can see a model that is neither stopping nor unloaded while its child is gone. --- tools/server/server-models.cpp | 45 ++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/tools/server/server-models.cpp b/tools/server/server-models.cpp index 181d689318..d661d99841 100644 --- a/tools/server/server-models.cpp +++ b/tools/server/server-models.cpp @@ -297,7 +297,7 @@ struct server_lru_sched { return; } queue.push_back({ model_id, 1, false }); - SRV_INF("models_max reached, request for name=%s queued at position %zu\n", + SRV_INF("request for name=%s queued at position %zu\n", model_id.c_str(), queue.size()); } @@ -1223,15 +1223,16 @@ void server_models::request_stop(const std::string & name, bool send_exit) { void server_models::on_child_exit(const std::string & name, const std::shared_ptr & proc, server_child_mode mode, int exit_code) { { std::lock_guard lk(mutex); - stopping_models.erase(name); auto it = mapping.find(name); if (it == mapping.end() || it->second.subproc != proc) { + stopping_models.erase(name); return; // entry erased, or a newer instance took the name } } if (mode == SERVER_CHILD_MODE_DOWNLOAD) { // instance will be cleaned up on next load_models() call std::lock_guard lk(mutex); + stopping_models.erase(name); cv.notify_all(); } else { update_status(name, { @@ -1301,6 +1302,9 @@ void server_models::update_status(const std::string & name, const update_status_ auto & meta = it->second.meta; meta.status = args.status; meta.exit_code = args.exit_code; + if (args.status == SERVER_MODEL_STATUS_UNLOADED) { + stopping_models.erase(name); + } if (!args.loaded_info.is_null()) { meta.loaded_info = args.loaded_info; } @@ -1440,10 +1444,15 @@ bool server_models::ensure_model_ready(const std::string & name, const std::func if (!meta.has_value()) { throw std::runtime_error("model name=" + name + " is not found"); } - if (meta->is_ready()) { + bool stopping; + { + std::lock_guard lk(mutex); + stopping = stopping_models.count(name) > 0; + } + if (!stopping && meta->is_ready()) { return false; // ready for taking requests } - if (meta->status == SERVER_MODEL_STATUS_SLEEPING) { + if (!stopping && meta->status == SERVER_MODEL_STATUS_SLEEPING) { return false; // child is sleeping but still running; new request will wake it up } @@ -1453,17 +1462,10 @@ bool server_models::ensure_model_ready(const std::string & name, const std::func std::unique_lock lk(mutex); auto it = mapping.find(name); if (it != mapping.end() && it->second.meta.status == SERVER_MODEL_STATUS_UNLOADED) { - if (sched->has_capacity(lk) && sched->queue_empty(lk)) { - lk.unlock(); - SRV_INF("model name=%s is not loaded, loading...\n", name.c_str()); - load(name); - did_load = true; - } else { - // also queue when a slot looks free but others wait already, else they starve - sched->join(lk, name); - sched->tick(lk); - queued = true; - } + // the queue entry protects the model from eviction until its waiters leave + sched->join(lk, name); + sched->tick(lk); + queued = true; } } @@ -1484,6 +1486,19 @@ bool server_models::ensure_model_ready(const std::string & name, const std::func if (it == mapping.end()) { break; // removed by another code path, nothing to wait for } + if (stopping_models.count(name)) { + // a stopping instance takes no new request, the next instance serves it + if (!queued) { + sched->join(lk, name); + sched->tick(lk); + queued = true; + } + if (should_stop && should_stop()) { + throw std::runtime_error("request cancelled while waiting for model name=" + name); + } + cv.wait_for(lk, std::chrono::milliseconds(200)); + continue; + } const server_model_status status = it->second.meta.status; if (status == SERVER_MODEL_STATUS_LOADED || status == SERVER_MODEL_STATUS_SLEEPING) {