fix(desktop): stop a live renderer before the POSIX stage-and-swap rename
The stage-and-swap promotion stops running desktop processes before renaming the live app aside, but _stop_desktop_processes_locking_build returns early off Windows: POSIX can rename a running app's files away, so the pack needs no lock release. A renderer left alive through the swap keeps fetching its OLD hashed chunks by path from the new tree and dies on the next lazy import with 'Failed to fetch dynamically imported module' — the app stays a broken UI until relaunch, even though the swapped-in bundle itself is complete (#85868; also reported as #96375 and #109643). Pass also_posix=True from the swap point only: the pack-time calls stay Windows-only because the staging pack never touches the live tree. Fixes #85868
This commit is contained in:
@@ -230,8 +230,12 @@ def _swap_staged_desktop_app(desktop_dir: Path, staging_dir: Path) -> Optional[P
|
||||
shutil.rmtree(previous, ignore_errors=True)
|
||||
moved_aside = live_root.exists()
|
||||
if moved_aside:
|
||||
# A Desktop may have reopened during the long packaging step.
|
||||
stopped = _stop_desktop_processes_locking_build(desktop_dir)
|
||||
# A Desktop may have reopened during the long packaging step (Windows lock) or
|
||||
# never exited at all (a manual `hermes update`/`hermes desktop` run does not
|
||||
# wait for it — only the update hand-offs do). Either way a renderer alive
|
||||
# past the rename below keeps fetching its old hashed chunks from disk and
|
||||
# dies on the next lazy import, so stop it on every platform (#109643).
|
||||
stopped = _stop_desktop_processes_locking_build(desktop_dir, also_posix=True)
|
||||
if stopped:
|
||||
logger.info("stopped desktop processes before staged app promotion: %s", stopped)
|
||||
_rename_riding_out_file_lock(live_root, previous)
|
||||
@@ -652,11 +656,15 @@ def _try_redownload_electron_dist(project_root: Path, env: dict) -> bool:
|
||||
return _redownload_electron_dist(project_root, env, mirror=_ELECTRON_FALLBACK_MIRROR)
|
||||
|
||||
|
||||
def _stop_desktop_processes_locking_build(desktop_dir: Path) -> list[int]:
|
||||
"""Terminate a running desktop app whose exe lives INSIDE this build's ``release`` tree (Windows
|
||||
only — its lock makes the pack die with ``Access is denied``; POSIX can unlink a running
|
||||
binary). Never raises; returns the PIDs asked to stop."""
|
||||
if sys.platform != "win32":
|
||||
def _stop_desktop_processes_locking_build(desktop_dir: Path, *, also_posix: bool = False) -> list[int]:
|
||||
"""Terminate a running desktop app whose exe lives INSIDE this build's ``release`` tree.
|
||||
|
||||
Windows needs it everywhere: the exe lock makes the pack die with ``Access is denied``.
|
||||
POSIX can rename a running app's files away, so the pack itself needs no stop — but a
|
||||
renderer left alive through the stage-and-swap promotion keeps fetching its OLD hashed
|
||||
chunks by path after the swap and dies on the next lazy import (#109643), so the swap
|
||||
point passes ``also_posix=True``. Never raises; returns the PIDs asked to stop."""
|
||||
if sys.platform != "win32" and not also_posix:
|
||||
return []
|
||||
try:
|
||||
import psutil
|
||||
|
||||
@@ -1567,6 +1567,64 @@ def test_swap_staged_desktop_app_rolls_back_when_second_rename_fails(tmp_path, m
|
||||
assert not (live_exe.parent.parent / (live_exe.parent.name + ".previous")).exists()
|
||||
|
||||
|
||||
def test_swap_staged_desktop_app_stops_live_renderer_before_rename(tmp_path):
|
||||
"""#109643: a renderer alive through the promotion rename keeps fetching its
|
||||
old hashed chunks from disk and dies on the next lazy import — the swap must
|
||||
ask for running desktop processes to stop on EVERY platform."""
|
||||
root = _make_desktop_tree(tmp_path)
|
||||
desktop_dir = root / "apps" / "desktop"
|
||||
live_exe = desktop_dir / "release" / _packaged_exe_rel()
|
||||
live_exe.parent.mkdir(parents=True)
|
||||
live_exe.write_text("old", encoding="utf-8")
|
||||
staging = main_desktop._desktop_staging_dir(desktop_dir)
|
||||
staged_exe = staging / _packaged_exe_rel()
|
||||
staged_exe.parent.mkdir(parents=True)
|
||||
staged_exe.write_text("new", encoding="utf-8")
|
||||
|
||||
with patch("hermes_cli.main_desktop._stop_desktop_processes_locking_build",
|
||||
return_value=[4321]) as stop:
|
||||
promoted = main_desktop._swap_staged_desktop_app(desktop_dir, staging)
|
||||
|
||||
assert promoted == live_exe
|
||||
stop.assert_called_once_with(desktop_dir, also_posix=True)
|
||||
|
||||
|
||||
def test_stop_desktop_processes_locking_build_posix_swap_bypasses_early_return(tmp_path, monkeypatch):
|
||||
"""#109643: also_posix=True must run the scan on POSIX (the default pack-time
|
||||
call stays Windows-only — the staging pack never touches the live tree)."""
|
||||
monkeypatch.setattr(main_desktop.sys, "platform", "darwin")
|
||||
root = _make_desktop_tree(tmp_path)
|
||||
desktop_dir = root / "apps" / "desktop"
|
||||
live_exe = desktop_dir / "release" / _packaged_exe_rel()
|
||||
live_exe.parent.mkdir(parents=True)
|
||||
live_exe.write_text("old", encoding="utf-8")
|
||||
|
||||
class _FakeProc:
|
||||
def __init__(self, pid, exe):
|
||||
self.info = {"pid": pid, "exe": exe}
|
||||
self.pid = pid
|
||||
|
||||
def terminate(self):
|
||||
return None
|
||||
|
||||
target = _FakeProc(100, str(live_exe))
|
||||
outsider = _FakeProc(200, "/usr/bin/unrelated")
|
||||
|
||||
class _FakePsutil:
|
||||
@staticmethod
|
||||
def process_iter(attrs):
|
||||
return [target, outsider]
|
||||
|
||||
@staticmethod
|
||||
def wait_procs(victims, timeout=5):
|
||||
return [], []
|
||||
|
||||
monkeypatch.setitem(sys.modules, "psutil", _FakePsutil)
|
||||
|
||||
assert main_desktop._stop_desktop_processes_locking_build(desktop_dir) == []
|
||||
assert main_desktop._stop_desktop_processes_locking_build(desktop_dir, also_posix=True) == [100]
|
||||
|
||||
|
||||
def test_gui_failed_pack_leaves_previous_app_untouched(tmp_path, monkeypatch, capsys):
|
||||
"""Every pack attempt fails → the pre-existing app is exactly as it was,
|
||||
no staging dir remains, exit is non-zero."""
|
||||
|
||||
Reference in New Issue
Block a user