From 023a5093dd6d0155857e63169e544fbab3cad9c1 Mon Sep 17 00:00:00 2001 From: joelteply Date: Mon, 12 Oct 2026 20:48:01 -0500 Subject: [PATCH 1/3] train: the start's claim is released on every exit that doesn't hand it to the worker, so an exception can't leave /train refusing every run until a restart Continuum card 48029d24, found reading the core's refusal contract (#5001). server_trainer::start claims the run (running CAS to true) and releases the claim through `refuse` on every explicit refusal. An exception thrown after the claim escaped with running still true: a json::type_error from a body field (e.g. "yield" read with body.value(.., true)), or std::system_error from std::thread. ex_wrapper answered 500, /train named the run "preparing" forever, and every later start was refused as "a training run is already in progress" until the server restarted. A claim_guard declared right after the claim releases it (state "error" under this out, running=false) unless `refuse` or the hand-off to the worker settled it. The explicit flag means it never releases a later run's claim. It's declared before `lock`, so `lock` (on mu) is gone before the guard takes mu. test-chat --train-control: a start with "yield": "not a bool" throws after the claim. The status is then "error" under its own out, and the next start is accepted. --- tests/test-chat.cpp | 25 +++++++++++++++++++++++++ tools/server/server-train.cpp | 27 +++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/tests/test-chat.cpp b/tests/test-chat.cpp index 201cea95d2c8..13fbc552a2fb 100644 --- a/tests/test-chat.cpp +++ b/tests/test-chat.cpp @@ -7277,6 +7277,31 @@ static int test_train_control(const char * model_path, const char * output_dir) require(trainer.start(req).at("ok").get(), "baseline refused"); const auto baseline = wait_for(done); + // what this catches (continuum card 48029d24): an exception thrown AFTER the claim. "yield" + // is read with body.value(..., true), so a string there throws json::type_error once the run + // is claimed and prepared. The claim must be released (state "error" under this out, and the + // next start accepted), never left "preparing" with every later start refused as "already in + // progress" until a restart. + { + json bad = req; + bad["out"] = "thrown.gguf"; + bad["yield"] = "not a bool"; + bool threw = false; + try { + trainer.start(bad); + } catch (const json::exception &) { + threw = true; + } + require(threw, "a non-bool yield must throw after the claim (the case under test)"); + const auto after = trainer.status(); + require(after.value("state", "") == "error" && after.value("out", "") == "thrown.gguf", + "the thrown start leaves its error under its own out, not 'preparing'"); + json next = req; + next["out"] = "after-throw.gguf"; + require(trainer.start(next).at("ok").get(), "the claim was released: the next start is accepted"); + wait_for(done); + } + // Block after two updates, then exercise manual pause separately from serving admission. calls.store(0); hold_at.store(3); diff --git a/tools/server/server-train.cpp b/tools/server/server-train.cpp index 8dd7f4d87426..e3a9cb94f1b3 100644 --- a/tools/server/server-train.cpp +++ b/tools/server/server-train.cpp @@ -505,6 +505,31 @@ json server_trainer::start(const json & body_in) { return json::object({{"ok", false}, {"error", "a training run is already in progress"}}); } } + // THE CLAIM IS RELEASED ON EVERY EXIT THAT DOES NOT HAND IT TO THE WORKER (continuum card + // 48029d24). `refuse` releases it on a refusal, but an exception thrown after the claim (a + // json::type_error from a body field, std::system_error from std::thread) used to escape with + // `running` still true: /train named this run "preparing" forever and refused every later + // start as "already in progress" until the server restarted. `settled` is set by `refuse` and + // by the hand-off; this guard releases only a claim nothing else settled, so it can never + // release a later run's claim. Declared before `lock` below, so `lock` (on `mu`) is gone + // before this destructor takes `mu`. + struct claim_guard { + server_trainer & trainer; + const json & body; + bool settled = false; + ~claim_guard() { + if (settled) { + return; + } + { + std::lock_guard claim_lock(trainer.mu); + trainer.state = json::object({{"state", "error"}, {"out", body.value("out", json())}, + {"error", "the start failed after it claimed the run (an exception); the claim is released"}, + {"epochs", json::array()}}); + } + trainer.running.store(false); + } + } claim{*this, body}; if (worker.joinable()) { worker.join(); } @@ -513,6 +538,7 @@ json server_trainer::start(const json & body_in) { state = json::object({{"state", "preparing"}, {"out", body.at("out")}, {"epochs", json::array()}}); } auto refuse = [&](const std::string & why) { + claim.settled = true; { std::lock_guard claim_lock(mu); state = json::object({{"state", "error"}, {"out", body.at("out")}, {"error", why}, {"epochs", json::array()}}); @@ -615,6 +641,7 @@ json server_trainer::start(const json & body_in) { window_ms_samples.clear(); window_ms_max = 0; worker = std::thread([this, body, ex = std::move(prepared)]() mutable { run(std::move(body), std::move(ex)); }); + claim.settled = true; // handed to the worker, which owns `running` from here lock.unlock(); return json::object({{"ok", true}, {"status", status()}}); } From 729d950bb85b9fd25f3f1f186ad466e6d676d9f9 Mon Sep 17 00:00:00 2001 From: joelteply Date: Mon, 12 Oct 2026 21:01:03 -0500 Subject: [PATCH 2/3] train: the claim rollback cannot throw, and a refusal settles the guard only after it released the claim Codex's review on #58 (023a5093d), request changes: - refuse set claim.settled first, before building the error state and clearing running. A throw there disarmed the rollback with running still true. settled is now set only after the release. - The guard's destructor built and assigned allocating JSON while unwinding. A throw there would hit the implicit noexcept and terminate the serving process. The error state is now built BEFORE the claim (out is validated above it), and the destructor (explicitly noexcept) only moves it in, inside try/catch with the lock, then releases running unconditionally. Recording the diagnostic is best-effort; releasing the claim is not. - test-chat --train-control also covers the refusal path: "examples": [] with no "text" is refused after the claim, leaves its error under its own out, and the next start is accepted. --- tests/test-chat.cpp | 15 +++++++++++++++ tools/server/server-train.cpp | 31 +++++++++++++++++++------------ 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/tests/test-chat.cpp b/tests/test-chat.cpp index 13fbc552a2fb..2bcedbe05108 100644 --- a/tests/test-chat.cpp +++ b/tests/test-chat.cpp @@ -7300,6 +7300,21 @@ static int test_train_control(const char * model_path, const char * output_dir) next["out"] = "after-throw.gguf"; require(trainer.start(next).at("ok").get(), "the claim was released: the next start is accepted"); wait_for(done); + + // and the refusal path: an empty "examples" is refused by `refuse` AFTER the claim, which + // must release it the same way (and settle the guard only once released) + json refused = req; + refused.erase("text"); + refused["examples"] = json::array(); + refused["out"] = "refused.gguf"; + require(!trainer.start(refused).at("ok").get(), "an empty examples array is refused"); + const auto after_refusal = trainer.status(); + require(after_refusal.value("state", "") == "error" && after_refusal.value("out", "") == "refused.gguf", + "the refusal leaves its error under its own out"); + json again = req; + again["out"] = "after-refusal.gguf"; + require(trainer.start(again).at("ok").get(), "the refused claim was released: the next start is accepted"); + wait_for(done); } // Block after two updates, then exercise manual pause separately from serving admission. diff --git a/tools/server/server-train.cpp b/tools/server/server-train.cpp index e3a9cb94f1b3..b8284ce46b5b 100644 --- a/tools/server/server-train.cpp +++ b/tools/server/server-train.cpp @@ -498,6 +498,11 @@ json server_trainer::start(const json & body_in) { // /train answers "preparing" with this out within milliseconds of the POST arriving, so a // caller reconciles by the run's identity instead of guessing from a clock. A refusal after the // claim releases it and leaves the run's own error, under its out, in the state. + // The state a claim released by an exception leaves behind, built BEFORE the claim (`out` + // is validated above) so releasing it later allocates nothing and cannot throw. + json failed_after_claim = json::object({{"state", "error"}, {"out", body.at("out")}, + {"error", "the start failed after it claimed the run (an exception); the claim is released"}, + {"epochs", json::array()}}); { std::lock_guard claim_lock(mu); bool expected = false; @@ -509,27 +514,29 @@ json server_trainer::start(const json & body_in) { // 48029d24). `refuse` releases it on a refusal, but an exception thrown after the claim (a // json::type_error from a body field, std::system_error from std::thread) used to escape with // `running` still true: /train named this run "preparing" forever and refused every later - // start as "already in progress" until the server restarted. `settled` is set by `refuse` and - // by the hand-off; this guard releases only a claim nothing else settled, so it can never - // release a later run's claim. Declared before `lock` below, so `lock` (on `mu`) is gone - // before this destructor takes `mu`. + // start as "already in progress" until the server restarted. `settled` is set only AFTER a + // release (by `refuse`) or by the hand-off, so this guard never releases a later run's claim + // and never misses one. Its destructor cannot throw (it runs while unwinding; a throw there + // terminates the server): the state is moved in, not built, recording it is best-effort, and + // `running` is released whatever happens. Declared before `lock` below, so `lock` (on `mu`) is + // gone before this destructor takes `mu`. struct claim_guard { server_trainer & trainer; - const json & body; + json failed_state; bool settled = false; - ~claim_guard() { + ~claim_guard() noexcept { if (settled) { return; } - { + try { std::lock_guard claim_lock(trainer.mu); - trainer.state = json::object({{"state", "error"}, {"out", body.value("out", json())}, - {"error", "the start failed after it claimed the run (an exception); the claim is released"}, - {"epochs", json::array()}}); + trainer.state = std::move(failed_state); + } catch (...) { + // the diagnostic state is best-effort; releasing the claim is not } trainer.running.store(false); } - } claim{*this, body}; + } claim{*this, std::move(failed_after_claim)}; if (worker.joinable()) { worker.join(); } @@ -538,12 +545,12 @@ json server_trainer::start(const json & body_in) { state = json::object({{"state", "preparing"}, {"out", body.at("out")}, {"epochs", json::array()}}); } auto refuse = [&](const std::string & why) { - claim.settled = true; { std::lock_guard claim_lock(mu); state = json::object({{"state", "error"}, {"out", body.at("out")}, {"error", why}, {"epochs", json::array()}}); } running.store(false); + claim.settled = true; // only once released: a throw above leaves the guard to release it return json::object({{"ok", false}, {"error", why}}); }; if (body.contains("examples")) { From 934864939b7c8ab8ecca16d8c03b8bca6b78f4ed Mon Sep 17 00:00:00 2001 From: joelteply Date: Mon, 12 Oct 2026 21:16:38 -0500 Subject: [PATCH 3/3] test-chat: catch the thrown start as std::exception, like the rest of the file ubuntu CI on 729d950bb: 'expected unqualified-id before &' at test-chat.cpp:7292. In this file json is common_json, which has no nested exception type, and every other catch here uses std::exception. nlohmann's type_error derives from it. --- tests/test-chat.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test-chat.cpp b/tests/test-chat.cpp index 2bcedbe05108..957771ee2c36 100644 --- a/tests/test-chat.cpp +++ b/tests/test-chat.cpp @@ -7289,7 +7289,7 @@ static int test_train_control(const char * model_path, const char * output_dir) bool threw = false; try { trainer.start(bad); - } catch (const json::exception &) { + } catch (const std::exception &) { // nlohmann's type_error is a std::exception; `json` here is common_json threw = true; } require(threw, "a non-bool yield must throw after the claim (the case under test)");