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