Stop an ignored-cancel sd-server, guard deletes during diffusion training, repair unusable managed binaries
sd-server does not interrupt an in-flight job, so when it ignores a cancel the grace branch abandoned the poll and reported cancellation while the native job kept a core (or the GPU) busy to completion and held the server's job slot. The comment said the caller stops the server, but only unload does that immediately: a superseding load stops it after its multi-gigabyte download, and a load that then fails never gets there. Stop it here, as the deadline branch already does. DELETE /api/models/delete-finetuned checked only the LLM trainer, so it could rmtree the output directory a live diffusion LoRA run was about to write its adapter into. Consult the diffusion training service too, like the dataset mutation and model-load routes. find_sd_*_binary only checks is_file(), so an interrupted extraction (or a prebuilt for the wrong CPU) left a present-but-unrunnable binary the installer never retried: every load probed it, fell back to diffusers, and native inference stayed off until the directory was deleted by hand. Probe it and reinstall, but only for a copy under the installer-owned root -- SD_CLI_PATH, UNSLOTH_SD_CPP_PATH, an in-tree build and anything on PATH are the user's.
This commit is contained in:
parent
7dbc95936e
commit
a2342f80df
7 changed files with 162 additions and 14 deletions
|
|
@ -65,6 +65,7 @@ from core.inference.sd_cpp_engine import (
|
|||
SdCppEngine,
|
||||
find_sd_cpp_binary,
|
||||
find_sd_server_binary,
|
||||
is_managed_binary,
|
||||
runtime_env,
|
||||
)
|
||||
from core.inference.sd_cpp_server import SdCppServer
|
||||
|
|
@ -123,6 +124,31 @@ def _server_binary_runnable(binary: str) -> bool:
|
|||
return proc.returncode >= 0 and proc.returncode not in (126, 127)
|
||||
|
||||
|
||||
def _usable_or_discard_managed(binary: str) -> bool:
|
||||
"""True if ``binary`` can be kept; False if it is an unusable copy WE own (now removed).
|
||||
|
||||
``find_sd_*_binary`` only checks that the path is a file, so an interrupted extraction (or a
|
||||
prebuilt for the wrong CPU) left a present-but-unrunnable binary that the installer then never
|
||||
retried: every load probed it, rejected it, and fell back to diffusers, so native inference
|
||||
stayed off until the user deleted the directory by hand. Probing here closes that loop.
|
||||
|
||||
Only a copy under the installer-owned root is removed. SD_CLI_PATH, UNSLOTH_SD_CPP_PATH, an
|
||||
in-tree build and anything on PATH are the user's, so an unrunnable one of those is reported
|
||||
as-is (the router still rejects it) rather than deleted or reinstalled over."""
|
||||
if _server_binary_runnable(binary):
|
||||
return True
|
||||
if not is_managed_binary(binary):
|
||||
logger.warning("sd.cpp binary %s is not runnable; leaving the user-supplied copy alone", binary)
|
||||
return True # not ours to replace; the router's own probe still refuses it
|
||||
logger.warning("managed sd.cpp binary %s is not runnable; removing it so it reinstalls", binary)
|
||||
try:
|
||||
Path(binary).unlink()
|
||||
except OSError as exc:
|
||||
logger.warning("could not remove the unusable managed binary %s: %s", binary, exc)
|
||||
return True # cannot repair it; don't spin on a reinstall that will find it again
|
||||
return False
|
||||
|
||||
|
||||
def ensure_sd_cpp_binary(*, allow_install: bool = True, accelerator: str = "cpu") -> Optional[str]:
|
||||
"""Path to a usable ``sd-cli`` binary, installing the prebuilt once if needed.
|
||||
|
||||
|
|
@ -131,14 +157,14 @@ def ensure_sd_cpp_binary(*, allow_install: bool = True, accelerator: str = "cpu"
|
|||
return is the router's signal to fall back to diffusers.
|
||||
"""
|
||||
found = find_sd_cpp_binary()
|
||||
if found:
|
||||
if found and _usable_or_discard_managed(found):
|
||||
return found
|
||||
if not allow_install:
|
||||
return None
|
||||
return found
|
||||
with _install_lock:
|
||||
# Re-check inside the lock: a concurrent first-load may have installed it.
|
||||
found = find_sd_cpp_binary()
|
||||
if found:
|
||||
if found and _usable_or_discard_managed(found):
|
||||
return found
|
||||
try:
|
||||
import sys
|
||||
|
|
@ -171,13 +197,13 @@ def ensure_sd_server_binary(
|
|||
uses the one-shot fallback. Never raises.
|
||||
"""
|
||||
found = find_sd_server_binary()
|
||||
if found:
|
||||
if found and _usable_or_discard_managed(found):
|
||||
return found
|
||||
if not allow_install:
|
||||
return None
|
||||
return found
|
||||
with _install_lock:
|
||||
found = find_sd_server_binary()
|
||||
if found:
|
||||
if found and _usable_or_discard_managed(found):
|
||||
return found
|
||||
try:
|
||||
import sys
|
||||
|
|
|
|||
|
|
@ -142,6 +142,30 @@ def _first_file(paths: list[Path]) -> Optional[str]:
|
|||
return None
|
||||
|
||||
|
||||
def managed_install_root() -> Path:
|
||||
"""The directory the prebuilt installer owns, so callers can tell a Studio-managed binary
|
||||
from a user-supplied one (SD_CLI_PATH / UNSLOTH_SD_CPP_PATH / PATH / an in-tree build).
|
||||
|
||||
Only a copy under this root may be reinstalled over: replacing anything else would delete
|
||||
a build the user chose. Honors UNSLOTH_STUDIO_HOME / STUDIO_HOME like the installer, so
|
||||
side-by-side Studios stay isolated."""
|
||||
studio_home = os.environ.get("UNSLOTH_STUDIO_HOME") or os.environ.get("STUDIO_HOME")
|
||||
if studio_home:
|
||||
return Path(studio_home).parent / "stable-diffusion.cpp"
|
||||
return Path.home() / ".unsloth" / "stable-diffusion.cpp"
|
||||
|
||||
|
||||
def is_managed_binary(binary: Optional[str]) -> bool:
|
||||
"""True when ``binary`` lives under the installer-owned root (see managed_install_root)."""
|
||||
if not binary:
|
||||
return False
|
||||
try:
|
||||
Path(binary).resolve().relative_to(managed_install_root().resolve())
|
||||
except (OSError, ValueError):
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
def _find_binary(
|
||||
*, direct_env: str, path_stems: tuple[str, ...], layout_stem: str
|
||||
) -> Optional[str]:
|
||||
|
|
@ -165,12 +189,7 @@ def _find_binary(
|
|||
|
||||
# 3. Default install root. Honors UNSLOTH_STUDIO_HOME / STUDIO_HOME like the installer, so
|
||||
# side-by-side Studios stay isolated; else ~/.unsloth/....
|
||||
studio_home = os.environ.get("UNSLOTH_STUDIO_HOME") or os.environ.get("STUDIO_HOME")
|
||||
default_root = (
|
||||
Path(studio_home).parent / "stable-diffusion.cpp"
|
||||
if studio_home
|
||||
else Path.home() / ".unsloth" / "stable-diffusion.cpp"
|
||||
)
|
||||
default_root = managed_install_root()
|
||||
hit = _first_file(_layout_candidates(default_root, layout_stem))
|
||||
if hit:
|
||||
return hit
|
||||
|
|
|
|||
|
|
@ -428,8 +428,16 @@ class SdCppServer:
|
|||
self.cancel(job_id)
|
||||
cancel_sent_at = time.monotonic()
|
||||
elif time.monotonic() - cancel_sent_at > _CANCEL_GRACE_S:
|
||||
# Cancel not reflected within the grace window; abandon the poll so the caller can stop the
|
||||
# server instead of holding the generate lock.
|
||||
# Cancel not reflected within the grace window, so the job is still running and
|
||||
# sd-server will not interrupt it (cancel_generating=false). Stop the process
|
||||
# before reporting the cancellation, exactly as the deadline branch below does:
|
||||
# abandoning the poll alone frees the generate lock while the native job keeps a
|
||||
# core (or the GPU) busy to completion and holds the server's job slot, so the
|
||||
# next request queues behind work nobody is waiting for. The caller does stop the
|
||||
# server on unload, but a SUPERSEDING LOAD only does so after its multi-gigabyte
|
||||
# download, and a load that then fails never reaches that point at all. Stopping
|
||||
# here is safe: the backend reloads on the next generate.
|
||||
self.stop()
|
||||
raise SdCppCancelled("sd-server generation was cancelled.")
|
||||
if not self.is_alive():
|
||||
# Unwinding a cancel (e.g. unload killed the server): clean cancellation.
|
||||
|
|
|
|||
|
|
@ -2526,6 +2526,20 @@ async def delete_finetuned_model(
|
|||
status_code = 409,
|
||||
detail = "Cannot delete trained models while training is running",
|
||||
)
|
||||
# The diffusion (Images) trainer is a second, independent run on the same storage root,
|
||||
# so checking only the LLM backend let a delete rmtree the output directory a live
|
||||
# diffusion run is about to write its adapter into: the run then dies at export, or
|
||||
# silently recreates part of the tree, and an expensive experiment is lost. The dataset
|
||||
# mutation and model-load routes already consult this service.
|
||||
from core.training.diffusion_training_service import get_diffusion_training_service
|
||||
if get_diffusion_training_service().is_active():
|
||||
raise HTTPException(
|
||||
status_code = 409,
|
||||
detail = (
|
||||
"Cannot delete trained models while diffusion (Images) training is "
|
||||
"running"
|
||||
),
|
||||
)
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
|
|
|
|||
|
|
@ -177,3 +177,29 @@ def test_an_unreadable_engine_state_fails_closed(client, monkeypatch):
|
|||
resp = _delete(c, target)
|
||||
assert resp.status_code == 503
|
||||
assert target.exists()
|
||||
|
||||
|
||||
def test_refuses_the_delete_while_a_diffusion_training_run_is_active(client, monkeypatch):
|
||||
# source="training" checked only the LLM trainer, so a delete could rmtree the output
|
||||
# directory a live diffusion LoRA run is about to write its adapter into.
|
||||
import sys
|
||||
import types
|
||||
|
||||
c, outputs = client
|
||||
target = _model_dir(outputs)
|
||||
monkeypatch.setattr(models_module, "_active_diffusion_backend", lambda: None)
|
||||
monkeypatch.setattr(models_module, "_active_video_backend", lambda: None)
|
||||
|
||||
stub = types.ModuleType("core.training.diffusion_training_service")
|
||||
stub.get_diffusion_training_service = lambda: types.SimpleNamespace(is_active = lambda: True)
|
||||
monkeypatch.setitem(sys.modules, "core.training.diffusion_training_service", stub)
|
||||
|
||||
resp = _delete(c, target)
|
||||
assert resp.status_code == 409
|
||||
assert "diffusion" in resp.json()["detail"].lower()
|
||||
assert target.exists()
|
||||
|
||||
# Idle again: the same delete goes through.
|
||||
stub.get_diffusion_training_service = lambda: types.SimpleNamespace(is_active = lambda: False)
|
||||
assert _delete(c, target).status_code == 200
|
||||
assert not target.exists()
|
||||
|
|
|
|||
|
|
@ -653,3 +653,54 @@ def test_print_asset_uses_upstream_fallback(monkeypatch, capsys):
|
|||
out = capsys.readouterr().out
|
||||
assert rc == 0
|
||||
assert "vulkan" in out and "no matching prebuilt" not in out
|
||||
|
||||
|
||||
def test_unrunnable_managed_binary_is_removed_so_it_reinstalls(monkeypatch, tmp_path):
|
||||
# An interrupted extraction leaves an sd-cli that exists but cannot run. The finder only checks
|
||||
# is_file(), so without a probe the installer never retried and native inference stayed off for
|
||||
# the life of the install.
|
||||
import core.inference.sd_cpp_backend as bk
|
||||
import core.inference.sd_cpp_engine as eng
|
||||
|
||||
root = tmp_path / "sd-home" / "stable-diffusion.cpp"
|
||||
root.mkdir(parents = True)
|
||||
managed = root / "sd-cli"
|
||||
managed.write_bytes(b"truncated")
|
||||
monkeypatch.setenv("UNSLOTH_STUDIO_HOME", str(tmp_path / "sd-home" / "studio"))
|
||||
assert eng.is_managed_binary(str(managed)) is True
|
||||
|
||||
monkeypatch.setattr(bk, "find_sd_cpp_binary", lambda: str(managed) if managed.exists() else None)
|
||||
monkeypatch.setattr(bk, "_server_binary_runnable", lambda *_a, **_k: False)
|
||||
installs: list = []
|
||||
|
||||
def _install(**kwargs):
|
||||
installs.append(kwargs)
|
||||
managed.write_bytes(b"good")
|
||||
return managed
|
||||
|
||||
import sys
|
||||
import types
|
||||
|
||||
stub = types.ModuleType("install_sd_cpp_prebuilt")
|
||||
stub.install = _install
|
||||
monkeypatch.setitem(sys.modules, "install_sd_cpp_prebuilt", stub)
|
||||
|
||||
out = bk.ensure_sd_cpp_binary(accelerator = "cpu")
|
||||
assert installs, "the unusable managed copy must trigger a reinstall"
|
||||
assert out == str(managed)
|
||||
|
||||
|
||||
def test_an_unrunnable_user_supplied_binary_is_never_deleted(monkeypatch, tmp_path):
|
||||
# SD_CLI_PATH / PATH / an in-tree build belong to the user: report them and let the router's own
|
||||
# probe refuse, but never remove or reinstall over them.
|
||||
import core.inference.sd_cpp_backend as bk
|
||||
|
||||
outside = tmp_path / "mine" / "sd-cli"
|
||||
outside.parent.mkdir(parents = True)
|
||||
outside.write_bytes(b"truncated")
|
||||
monkeypatch.setenv("UNSLOTH_STUDIO_HOME", str(tmp_path / "elsewhere" / "studio"))
|
||||
monkeypatch.setattr(bk, "find_sd_cpp_binary", lambda: str(outside))
|
||||
monkeypatch.setattr(bk, "_server_binary_runnable", lambda *_a, **_k: False)
|
||||
|
||||
assert bk.ensure_sd_cpp_binary(accelerator = "cpu") == str(outside)
|
||||
assert outside.exists(), "a user-supplied binary must survive"
|
||||
|
|
|
|||
|
|
@ -366,6 +366,10 @@ def test_img_gen_abandons_when_cancel_not_honored(patched):
|
|||
s = _server_with(popen, client)
|
||||
with pytest.raises(SdCppCancelled):
|
||||
s.img_gen({"prompt": "x"}, cancel_event = cancel, poll_interval = 0.01)
|
||||
# And the process is stopped, not left running the abandoned job: sd-server does not interrupt
|
||||
# an in-flight job, so a server that ignored the cancel would otherwise burn a core (or the GPU)
|
||||
# to completion and hold its job slot against the next request.
|
||||
assert not s.is_alive()
|
||||
|
||||
|
||||
def test_img_gen_non_dict_submit_json_raises(patched):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue