fix: windows hand-off keeps the marker while the checkout lock is held (R6)
Release waits (bounded) while any process still holds the checkout lock's byte (hermes_cli/update_lock.py takes it with msvcrt.locking at 1 MiB), and -MarkerOp reclaim answers `held` instead of freeing a dead marker in that state, so a Desktop gate never opens over a surviving completion. posix.sh withdraw now keeps a dead adopter's claim (`foreign`), like marker.ps1. (cherry picked from commit 8a5525d26f036a3c4b5176f11ee4a590c746e2db)
This commit is contained in:
@@ -188,6 +188,18 @@ function Invoke-MarkerRelease {
|
||||
# Corpus 'release' under the lock: owner deletes, or hands the claim over
|
||||
# to a live delegate; anything that is not ours is kept.
|
||||
if ($NoMarkerCleanup -or $script:MarkerClaim -notin @('claimed', 'adopted')) { return }
|
||||
# R6: never while a survivor of the update still holds the checkout lock --
|
||||
# the marker would read free while that process still mutates the checkout.
|
||||
$waited = 0
|
||||
while (Test-CheckoutLockHeld) {
|
||||
if ($waited -eq 0) { Write-HandoffLog "a process still holds the checkout update lock; keeping the update marker until it exits" }
|
||||
if ($waited -ge $script:MarkerReleaseWaitSeconds) {
|
||||
Write-HandoffLog "checkout update lock still held after $($waited)s; leaving the update marker"
|
||||
return
|
||||
}
|
||||
Start-Sleep -Seconds 1
|
||||
$waited++
|
||||
}
|
||||
$lock = Open-MarkerLock
|
||||
if ($null -eq $lock) { Write-HandoffLog "update marker lock stayed busy; leaving the marker to identity-checking readers"; return }
|
||||
try {
|
||||
@@ -236,7 +248,7 @@ function Update-MarkerHeartbeat {
|
||||
|
||||
function Invoke-MarkerOp([string]$Op, [int]$Desktop, [string]$Run) {
|
||||
# SPEC 6 helper ops (Electron's only way to mutate). Returns the verdict
|
||||
# line: absent | reclaimed | live <pid> | busy | withdrawn | taken <pid> | foreign.
|
||||
# line: absent | reclaimed | held | live <pid> | busy | withdrawn | taken <pid> | foreign.
|
||||
$lock = Open-MarkerLock
|
||||
if ($null -eq $lock) { return 'busy' }
|
||||
try {
|
||||
@@ -248,6 +260,7 @@ function Invoke-MarkerOp([string]$Op, [int]$Desktop, [string]$Run) {
|
||||
$j = $read.Judgement
|
||||
if ($Op -eq 'reclaim') {
|
||||
if ($j.Verdict -in @('live', 'ours')) { return "live $($j.Owner)" }
|
||||
if (Test-CheckoutLockHeld) { return 'held' } # R6: a survivor still mutates the checkout
|
||||
if (-not (Remove-MarkerLocked)) { return 'busy' }
|
||||
Write-HandoffLog "marker-op reclaim: removed a $($j.Verdict) update marker"
|
||||
return 'reclaimed'
|
||||
|
||||
@@ -29,6 +29,7 @@ $script:MarkerCtTolerance = 2.0
|
||||
$script:MarkerOwnCtEpsilon = 0.005
|
||||
$script:MarkerLockTimeoutMs = 10000
|
||||
$script:MarkerHeartbeatSeconds = 300
|
||||
$script:MarkerReleaseWaitSeconds = 7200
|
||||
$script:ProcessCtCache = @{}
|
||||
$script:MarkerClaim = "none" # claimed | adopted | refused
|
||||
$script:MarkerBlocker = 0
|
||||
@@ -330,3 +331,50 @@ function Read-MarkerLocked($Ctx) {
|
||||
}
|
||||
|
||||
. (Join-Path $PSScriptRoot 'marker-claim.ps1')
|
||||
|
||||
function Get-CheckoutLockPath {
|
||||
# hermes_cli/update_lock.py::checkout_lock_path: <git common dir>/hermes-update.lock,
|
||||
# else <root>/.hermes-update.lock.
|
||||
$root = Get-Variable -Name InstallRoot -ValueOnly -ErrorAction SilentlyContinue
|
||||
if (-not $root) { return $null }
|
||||
$dot = Join-Path $root '.git'
|
||||
$gitDir = $null
|
||||
if ([System.IO.Directory]::Exists($dot)) {
|
||||
$gitDir = $dot
|
||||
} elseif ([System.IO.File]::Exists($dot)) {
|
||||
$first = (([System.IO.File]::ReadAllText($dot)).TrimStart([char]0xFEFF) -split "`n")[0].Trim()
|
||||
if ($first -like 'gitdir:*') {
|
||||
$gitDir = $first.Substring(7).Trim()
|
||||
if (-not [System.IO.Path]::IsPathRooted($gitDir)) { $gitDir = Join-Path $root $gitDir }
|
||||
}
|
||||
}
|
||||
if (-not $gitDir) { return (Join-Path $root '.hermes-update.lock') }
|
||||
$commonFile = Join-Path $gitDir 'commondir'
|
||||
if ([System.IO.File]::Exists($commonFile)) {
|
||||
$common = (([System.IO.File]::ReadAllText($commonFile)).TrimStart([char]0xFEFF) -split "`n")[0].Trim()
|
||||
if ($common) {
|
||||
$gitDir = if ([System.IO.Path]::IsPathRooted($common)) { $common } else { Join-Path $gitDir $common }
|
||||
}
|
||||
}
|
||||
return (Join-Path $gitDir 'hermes-update.lock')
|
||||
}
|
||||
|
||||
function Test-CheckoutLockHeld {
|
||||
# R6: does some process hold the checkout lock right now? Python takes it with
|
||||
# msvcrt.locking on one byte at offset 1 MiB (LockFile); probe the same byte and
|
||||
# give it straight back.
|
||||
$path = Get-CheckoutLockPath
|
||||
if (-not $path -or -not [System.IO.File]::Exists($path)) { return $false }
|
||||
$fs = $null
|
||||
try {
|
||||
$fs = [System.IO.File]::Open($path, [System.IO.FileMode]::Open, [System.IO.FileAccess]::Read,
|
||||
([System.IO.FileShare]::ReadWrite -bor [System.IO.FileShare]::Delete))
|
||||
} catch { return $false }
|
||||
try {
|
||||
try { $fs.Lock(1048576, 1) } catch [System.IO.IOException] { return $true }
|
||||
try { $fs.Unlock(1048576, 1) } catch {}
|
||||
return $false
|
||||
} finally {
|
||||
$fs.Dispose()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -479,7 +479,7 @@ marker_op_withdraw_locked() {
|
||||
marker_judge "$SEEN"
|
||||
if [ -z "$M_PID" ] || [ "$M_RUN" != "$HANDOFF_RUN" ]; then echo foreign; return 0; fi
|
||||
if [ "$M_PID" != "$DESKTOP_PID" ] && [ "$J_OWNER_STATE" -eq 2 ]; then echo "taken $M_PID"; return 0; fi
|
||||
if [ "$M_PID" = "$DESKTOP_PID" ] || [ "$J_VERDICT" = dead ]; then
|
||||
if [ "$M_PID" = "$DESKTOP_PID" ] && [ "$J_OWNER_STATE" -eq 2 ] && [ -n "$M_CT" ]; then
|
||||
rm -f "$MARKER" 2>/dev/null
|
||||
log "withdrew the Desktop's hand-off marker (run $HANDOFF_RUN)"
|
||||
echo withdrawn; return 0
|
||||
@@ -487,7 +487,10 @@ marker_op_withdraw_locked() {
|
||||
echo foreign
|
||||
}
|
||||
|
||||
marker_op() { # reclaim | withdraw -> one verdict line on stdout
|
||||
marker_op() { # reclaim | withdraw -> one verdict line on stdout (same words as marker.ps1):
|
||||
# reclaim absent | live <pid> | busy | held (dead, but the checkout lock is held) | reclaimed
|
||||
# withdraw absent | taken <pid> (a live adopter carries the run) | withdrawn (this Desktop's
|
||||
# own bridge, removed) | foreign (anything else, kept -- a dead adopter included)
|
||||
local rc
|
||||
case "$1" in
|
||||
reclaim) marker_locked marker_op_reclaim_locked ;;
|
||||
|
||||
@@ -145,3 +145,39 @@ def test_heartbeat_refreshes_line_2_only_for_our_own_claim(tmp_path: Path, sleep
|
||||
foreign = f'{sleeper.pid}\n{started}\nct:{dct}\n'
|
||||
_harness(tmp_path, foreign, 'heartbeat')
|
||||
assert (tmp_path / MARKER).read_bytes().decode() == foreign
|
||||
|
||||
|
||||
_CHECKOUT_HOLDER = """
|
||||
import msvcrt, os, sys, time
|
||||
from pathlib import Path
|
||||
fd = os.open(sys.argv[1], os.O_RDWR | os.O_CREAT | os.O_BINARY, 0o644)
|
||||
os.lseek(fd, 1 << 20, os.SEEK_SET)
|
||||
msvcrt.locking(fd, msvcrt.LK_NBLCK, 1) # exactly hermes_cli/update_lock.py::_try_lock
|
||||
Path(sys.argv[2] + '.ready').write_text('1', encoding='utf-8')
|
||||
while not Path(sys.argv[2]).exists():
|
||||
time.sleep(0.05)
|
||||
"""
|
||||
|
||||
|
||||
@pytest.mark.platforms('windows')
|
||||
def test_marker_op_reclaim_reports_held_while_a_survivor_holds_the_checkout_lock(tmp_path: Path) -> None:
|
||||
"""R6: the update died (dead marker) but a process it started still holds the checkout lock:
|
||||
the Desktop's reclaim must not free the marker; it answers `held` until that process exits."""
|
||||
install = tmp_path / 'hermes-agent'
|
||||
install.mkdir()
|
||||
dead = f'{_dead_pid()}\n{int(time.time())}\nct:5.000\n'.encode()
|
||||
(tmp_path / MARKER).write_bytes(dead)
|
||||
release = tmp_path / 'release-holder'
|
||||
holder = subprocess.Popen([sys.executable, '-c', _CHECKOUT_HOLDER, str(install / '.hermes-update.lock'), str(release)])
|
||||
try:
|
||||
deadline = time.monotonic() + 30
|
||||
while not Path(str(release) + '.ready').exists():
|
||||
assert time.monotonic() < deadline and holder.poll() is None
|
||||
time.sleep(0.05)
|
||||
assert _op(tmp_path, '-MarkerOp', 'reclaim')[:2] == (0, 'held\n')
|
||||
assert (tmp_path / MARKER).read_bytes() == dead
|
||||
finally:
|
||||
release.touch()
|
||||
holder.wait(timeout=30)
|
||||
assert _op(tmp_path, '-MarkerOp', 'reclaim')[:2] == (0, 'reclaimed\n')
|
||||
assert not (tmp_path / MARKER).exists()
|
||||
|
||||
Reference in New Issue
Block a user