fix(studio): fall back to copy when os.replace is blocked during install activation (#6133)
* fix(studio): fall back to copy when os.replace is blocked during install activation On Windows ARM64 the antivirus scanner can transiently hold a freshly extracted DLL open while MoveFileEx runs, so activating the staged llama.cpp prebuilt fails with [WinError 5] Access is denied. Attempt os.replace first, then fall back to a file-by-file copytree which bypasses the rename. * address review: keep os.replace for rollback, scope copy-fallback to staging The copy + rmtree fallback could silently corrupt a live install if the existing directory is busy. Restrict it to freshly extracted staging trees (renamed activate_staged_dir) and keep strict os.replace for the rollback move so a busy active install raises immediately. * fix(studio): scope copy-fallback to busy-lock errors, log it, and add tests --------- Co-authored-by: Lee Jackson <130007945+Imagineer99@users.noreply.github.com> Co-authored-by: Etherll <61019402+Etherll@users.noreply.github.com>
This commit is contained in:
parent
c3604d01f7
commit
a22169941d
2 changed files with 74 additions and 1 deletions
|
|
@ -4870,6 +4870,35 @@ def confirm_install_tree(install_dir: Path, host: HostInfo) -> None:
|
|||
raise RuntimeError("activated install was missing expected files: " + ", ".join(missing))
|
||||
|
||||
|
||||
def activate_staged_dir(staging_dir: Path, dst: Path) -> None:
|
||||
"""Move a freshly extracted ``staging_dir`` onto ``dst``.
|
||||
|
||||
``os.replace`` is attempted first as the fast path. On Windows ARM64 the
|
||||
antivirus scanner can transiently hold a freshly extracted DLL open at the
|
||||
moment ``MoveFileEx`` runs, surfacing as ``[WinError 5] Access is denied``;
|
||||
a file-by-file copy bypasses the rename entirely.
|
||||
|
||||
This fallback is intentionally limited to staging trees we just extracted.
|
||||
It must not be used to move an existing/active install aside: there an
|
||||
``os.replace`` failure means the directory is genuinely in use, and a
|
||||
silent copy + ``rmtree`` could partially delete a live install.
|
||||
|
||||
Only busy/lock errors (``is_busy_lock_error``) trigger the copy; anything
|
||||
else (disk full, cross-device, missing path) re-raises so it cannot leave
|
||||
a partially copied install behind. A copy is preferred over retrying the
|
||||
rename because antivirus scans of large DLLs can outlast any reasonable
|
||||
retry window.
|
||||
"""
|
||||
try:
|
||||
os.replace(staging_dir, dst)
|
||||
except OSError as exc:
|
||||
if not is_busy_lock_error(exc):
|
||||
raise
|
||||
log(f"os.replace failed ({exc!r}); falling back to file-by-file copy of staging tree")
|
||||
shutil.copytree(staging_dir, dst, dirs_exist_ok = True)
|
||||
remove_tree(staging_dir)
|
||||
|
||||
|
||||
def activate_install_tree(staging_dir: Path, install_dir: Path, host: HostInfo) -> None:
|
||||
rollback_dir: Path | None = None
|
||||
failed_dir: Path | None = None
|
||||
|
|
@ -4881,7 +4910,7 @@ def activate_install_tree(staging_dir: Path, install_dir: Path, host: HostInfo)
|
|||
log(f"moved existing install to rollback path {rollback_dir.name}")
|
||||
|
||||
log(f"activating staged install {staging_dir} -> {install_dir}")
|
||||
os.replace(staging_dir, install_dir)
|
||||
activate_staged_dir(staging_dir, install_dir)
|
||||
log(f"activated staged install at {install_dir}")
|
||||
log(f"confirming activated install tree at {install_dir}")
|
||||
confirm_install_tree(install_dir, host)
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import errno
|
||||
import importlib.util
|
||||
import io
|
||||
import json
|
||||
|
|
@ -28,6 +29,7 @@ ApprovedReleaseChecksums = INSTALL_LLAMA_PREBUILT.ApprovedReleaseChecksums
|
|||
hydrate_source_tree = INSTALL_LLAMA_PREBUILT.hydrate_source_tree
|
||||
validate_prebuilt_choice = INSTALL_LLAMA_PREBUILT.validate_prebuilt_choice
|
||||
activate_install_tree = INSTALL_LLAMA_PREBUILT.activate_install_tree
|
||||
activate_staged_dir = INSTALL_LLAMA_PREBUILT.activate_staged_dir
|
||||
create_install_staging_dir = INSTALL_LLAMA_PREBUILT.create_install_staging_dir
|
||||
sha256_file = INSTALL_LLAMA_PREBUILT.sha256_file
|
||||
source_archive_logical_name = INSTALL_LLAMA_PREBUILT.source_archive_logical_name
|
||||
|
|
@ -666,6 +668,48 @@ def test_activate_install_tree_cleans_all_paths_when_rollback_restore_fails(
|
|||
assert "removing rollback path" in output
|
||||
|
||||
|
||||
def test_activate_staged_dir_copies_when_replace_hits_busy_lock(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
|
||||
):
|
||||
staging_dir = tmp_path / "llama.cpp.staging-test"
|
||||
(staging_dir / "bin").mkdir(parents = True)
|
||||
(staging_dir / "bin" / "ggml-base.dll").write_bytes(b"fake dll")
|
||||
dst = tmp_path / "llama.cpp"
|
||||
|
||||
def denied_replace(src, dst_arg):
|
||||
raise PermissionError(errno.EACCES, "Access is denied", str(src))
|
||||
|
||||
monkeypatch.setattr(INSTALL_LLAMA_PREBUILT.os, "replace", denied_replace)
|
||||
|
||||
activate_staged_dir(staging_dir, dst)
|
||||
|
||||
assert (dst / "bin" / "ggml-base.dll").read_bytes() == b"fake dll"
|
||||
assert not staging_dir.exists()
|
||||
|
||||
captured = capsys.readouterr()
|
||||
assert "falling back to file-by-file copy" in captured.out + captured.err
|
||||
|
||||
|
||||
def test_activate_staged_dir_reraises_non_busy_errors(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
):
|
||||
staging_dir = tmp_path / "llama.cpp.staging-test"
|
||||
staging_dir.mkdir()
|
||||
(staging_dir / "new.txt").write_text("new install\n")
|
||||
dst = tmp_path / "llama.cpp"
|
||||
|
||||
def out_of_space_replace(src, dst_arg):
|
||||
raise OSError(errno.ENOSPC, "No space left on device", str(src))
|
||||
|
||||
monkeypatch.setattr(INSTALL_LLAMA_PREBUILT.os, "replace", out_of_space_replace)
|
||||
|
||||
with pytest.raises(OSError, match = "No space left on device"):
|
||||
activate_staged_dir(staging_dir, dst)
|
||||
|
||||
assert not dst.exists()
|
||||
assert (staging_dir / "new.txt").read_text() == "new install\n"
|
||||
|
||||
|
||||
def test_binary_env_linux_includes_binary_parent_in_ld_library_path(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue