Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions tests/test-chat.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7277,6 +7277,46 @@ static int test_train_control(const char * model_path, const char * output_dir)
require(trainer.start(req).at("ok").get<bool>(), "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 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)");
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<bool>(), "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<bool>(), "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<bool>(), "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.
calls.store(0);
hold_at.store(3);
Expand Down
34 changes: 34 additions & 0 deletions tools/server/server-train.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -498,13 +498,45 @@ 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<std::mutex> claim_lock(mu);
bool expected = false;
if (!running.compare_exchange_strong(expected, true)) {
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 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;
json failed_state;
bool settled = false;
~claim_guard() noexcept {
if (settled) {
return;
}
try {
std::lock_guard<std::mutex> claim_lock(trainer.mu);
trainer.state = std::move(failed_state);
} catch (...) {
// the diagnostic state is best-effort; releasing the claim is not
}
trainer.running.store(false);
}
} claim{*this, std::move(failed_after_claim)};
if (worker.joinable()) {
worker.join();
}
Expand All @@ -518,6 +550,7 @@ json server_trainer::start(const json & body_in) {
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")) {
Expand Down Expand Up @@ -615,6 +648,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()}});
}
Expand Down
Loading