From c8ac525cbfa7e5f5cd3ab9e4d975ff998d94c900 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 5 Oct 2026 01:42:44 -0700 Subject: [PATCH] fix(update): Windows marker decisions re-read creation times instead of a pid's cached one Get-LiveProcessCt caches a pid's creation time for the script's lifetime and forgets it only when a probe SEES the pid dead. A delegate (the `hermes update` child) that exits and whose pid is reused between two probes keeps the old incarnation's time, so the release judged the unrelated new process live and handed the marker to it (A7 rule 5), parking every later update behind it. Every judgement made through New-MarkerContext (claim, delegate, heartbeat, release) and the delegate line Add-MarkerDelegate writes now pass -Fresh, which re-reads the time. Pollers that only ask "still alive?" (Wait-DesktopExit) keep the cache. Test (Windows live cell): a release with the delegate's pid cached at the recorded ct but really owned by another incarnation deletes the marker instead of handing it over. marker.sh reads /proc or ps on every call: no cache. review S6 --- scripts/desktop-update/marker-claim.ps1 | 2 +- scripts/desktop-update/marker.ps1 | 12 ++++++---- .../test_desktop_update_windows_marker_ops.py | 24 ++++++++++++++++--- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/scripts/desktop-update/marker-claim.ps1 b/scripts/desktop-update/marker-claim.ps1 index 02f7b8dfa9..181208456d 100644 --- a/scripts/desktop-update/marker-claim.ps1 +++ b/scripts/desktop-update/marker-claim.ps1 @@ -201,7 +201,7 @@ function Add-MarkerDelegate([int[]]$Candidates) { } foreach ($candidate in @($Candidates)) { if ($candidate -le 0 -or $candidate -eq $PID) { continue } - $probe = Get-LiveProcessCt $candidate + $probe = Get-LiveProcessCt $candidate -Fresh if (-not $probe.Alive -or $null -eq $probe.Ct) { continue } $line = "delegate:$candidate ct:$(Format-Ct $probe.Ct)" if (-not (Set-MarkerBodyLocked (Format-MarkerBody $info.Pid $info.StartedAtText $info.CtText $line $info.Runs))) { return 'lost' } diff --git a/scripts/desktop-update/marker.ps1 b/scripts/desktop-update/marker.ps1 index 94140c9725..a6f4f1c410 100644 --- a/scripts/desktop-update/marker.ps1 +++ b/scripts/desktop-update/marker.ps1 @@ -67,10 +67,12 @@ function Get-ParentProcessId([int]$ProcessId) { return 0 } -function Get-LiveProcessCt([int64]$ProcessId) { +function Get-LiveProcessCt([int64]$ProcessId, [switch]$Fresh) { # Alive + creation time (unix seconds, $null when unreadable). The time is # read once per pid and kept while that pid stays alive: a waiter polls - # liveness, never one CIM query per poll. + # liveness, never one CIM query per poll. The cache only forgets a pid it + # SAW dead, so a pid that exited and was reused between two polls keeps the + # old incarnation's time: every marker decision passes -Fresh (re-read). if ($ProcessId -le 0 -or $ProcessId -gt [int]::MaxValue) { return @{ Alive = $false; Ct = $null } } $id = [int]$ProcessId $p = Get-Process -Id $id -ErrorAction SilentlyContinue @@ -80,7 +82,7 @@ function Get-LiveProcessCt([int64]$ProcessId) { $script:ProcessCtCache.Remove($id) return @{ Alive = $false; Ct = $null } } - if (-not $script:ProcessCtCache.ContainsKey($id)) { + if ($Fresh -or -not $script:ProcessCtCache.ContainsKey($id)) { $script:ProcessCtCache[$id] = Get-ProcessCreationCt $id } return @{ Alive = $true; Ct = $script:ProcessCtCache[$id] } @@ -193,8 +195,10 @@ function Get-MarkerDelegateLine($Info) { function New-MarkerContext { # Who "we" are and how a pid is probed; the corpus test injects its own. + # Fresh creation times: a judgement here decides a claim, a delegate or a + # hand-over, which must never ride a reused pid's cached identity. $own = Get-LiveProcessCt $PID - return @{ OwnPid = [int64]$PID; OwnCt = $own.Ct; Now = (Get-UnixNow); Probe = { param($p) Get-LiveProcessCt $p } } + return @{ OwnPid = [int64]$PID; OwnCt = $own.Ct; Now = (Get-UnixNow); Probe = { param($p) Get-LiveProcessCt $p -Fresh } } } function Get-MarkerIdentityState([int64]$ProcessId, $RecordedCt, [int64]$StartedAt, $Ctx) { diff --git a/tests/scripts/desktop_update/test_desktop_update_windows_marker_ops.py b/tests/scripts/desktop_update/test_desktop_update_windows_marker_ops.py index 51b2fdf608..32fa4b74a9 100644 --- a/tests/scripts/desktop_update/test_desktop_update_windows_marker_ops.py +++ b/tests/scripts/desktop_update/test_desktop_update_windows_marker_ops.py @@ -90,12 +90,17 @@ def test_marker_op_withdraw(tmp_path: Path, sleeper: subprocess.Popen) -> None: # -- release / heartbeat (production functions in a real PowerShell process) --- HARNESS = r""" -param([string]$MarkerPs1, [string]$Marker, [string]$Body, [string]$Action, [string]$InstallRoot = '', [int]$Heartbeat = 300) +param([string]$MarkerPs1, [string]$Marker, [string]$Body, [string]$Action, [string]$InstallRoot = '', [int]$Heartbeat = 300, + [string]$StaleCt = '') $MarkerPath = $Marker $NoMarkerCleanup = $false function Write-HandoffLog([string]$Message) { [Console]::Error.WriteLine($Message) } . $MarkerPs1 $script:MarkerHeartbeatSeconds = $Heartbeat +if ($StaleCt) { # ":": what the cache kept for an earlier incarnation of that pid + $stale = $StaleCt.Split(':') + $script:ProcessCtCache[[int]$stale[0]] = [double]::Parse($stale[1], [Globalization.CultureInfo]::InvariantCulture) +} $own = Format-Ct (Get-LiveProcessCt $PID).Ct [System.IO.File]::WriteAllText($MarkerPath, $Body.Replace('{self}', "$PID").Replace('{selfct}', $own)) $script:MarkerClaim = 'claimed' @@ -105,12 +110,12 @@ if ($Action -eq 'heartbeat') { $script:MarkerHeartbeatSeconds = 0; Update-Marker """ -def _harness(tmp_path: Path, body: str, action: str) -> tuple[str, str]: +def _harness(tmp_path: Path, body: str, action: str, *extra: str) -> tuple[str, str]: harness = tmp_path / 'harness.ps1' harness.write_text(HARNESS, encoding='utf-8') proc = subprocess.run([POWERSHELL, '-NoProfile', '-ExecutionPolicy', 'Bypass', '-File', str(harness), '-MarkerPs1', str(MARKER_PS1), '-Marker', str(tmp_path / MARKER), - '-Body', body, '-Action', action], + '-Body', body, '-Action', action, *extra], capture_output=True, text=True, timeout=120) assert proc.returncode == 0, proc.stdout + proc.stderr pid, ct = proc.stdout.split() @@ -134,6 +139,19 @@ def test_release_hands_the_claim_to_a_live_delegate( assert (tmp_path / MARKER).read_bytes().decode() == f'{dpid}\n{started}\nct:{dct}\nrun:desk-3\n' +@pytest.mark.platforms('windows') +def test_release_rereads_a_cached_delegate_identity_before_handing_over( + tmp_path: Path, sleeper: subprocess.Popen, +) -> None: + """Get-LiveProcessCt forgets a pid only when it SEES it dead: a delegate that exited and + whose pid was reused between two polls keeps its cached creation time. The release must + re-read it, or the marker is handed to the unrelated process now wearing that pid.""" + started = int(time.time()) - 60 + stale = f'{{self}}\n{started}\nct:{{selfct}}\ndelegate:{sleeper.pid} ct:5.000\nrun:desk-3\n' + _harness(tmp_path, stale, 'release', '-StaleCt', f'{sleeper.pid}:5.0') + assert not (tmp_path / MARKER).exists(), (tmp_path / MARKER).read_bytes() + + @pytest.mark.platforms('windows') def test_heartbeat_refreshes_line_2_only_for_our_own_claim(tmp_path: Path, sleeper: subprocess.Popen) -> None: started = int(time.time()) - 900