Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions cmd/agentdock-shim/setup_runtime_host_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (
"unicode/utf8"

"golang.org/x/sys/windows"

processctl "github.com/uvwt/agentdock/internal/process"
)

func runSetupRuntimeHost(args []string) (int, error) {
Expand Down Expand Up @@ -142,12 +144,30 @@ func launchSetupRuntimeProcess(filePath, arguments, agentDockHome, agentDockDefa
defer stderrFile.Close()
command.Stdout = stdoutFile
command.Stderr = stderrFile
if err := command.Run(); err != nil {
if err := command.Start(); err != nil {
return 1, fmt.Errorf("start setup runtime process: %w", err)
}
controller, err := processctl.Attach(command)
if err != nil {
_ = command.Process.Kill()
_ = command.Wait()
return 1, fmt.Errorf("supervise setup runtime process: %w", err)
}
defer controller.Close()

if err := command.Wait(); err != nil {
var exitErr *exec.ExitError
if errors.As(err, &exitErr) {
return exitErr.ExitCode(), nil
}
return 1, fmt.Errorf("run setup runtime process: %w", err)
return 1, fmt.Errorf("wait for setup runtime process: %w", err)
}

// service start may intentionally leave the real Core alive after the short-lived
// command exits. Disarm kill-on-close only after a successful exit. If Task Scheduler
// cancels this host before then, the OS closes the Job handle and kills the whole tree.
if err := controller.Detach(); err != nil {
return 1, fmt.Errorf("detach successful setup runtime process: %w", err)
}
return 0, nil
}
Expand Down
42 changes: 42 additions & 0 deletions internal/process/process_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,48 @@ func (c *Controller) Terminate() error {
return c.terminateErr
}

// Detach removes kill-on-close ownership before closing the controller handle.
// It is used when a supervised launcher completed successfully and intentionally
// left long-lived descendants behind. If the owner process dies before Detach,
// Windows still closes the Job handle with KILL_ON_JOB_CLOSE and terminates them.
func (c *Controller) Detach() error {
if c == nil {
return nil
}
c.mu.Lock()
defer c.mu.Unlock()
if c.job == 0 {
return nil
}

limits := windows.JOBOBJECT_EXTENDED_LIMIT_INFORMATION{}
var returned uint32
if err := windows.QueryInformationJobObject(
c.job,
windows.JobObjectExtendedLimitInformation,
uintptr(unsafe.Pointer(&limits)),
uint32(unsafe.Sizeof(limits)),
&returned,
); err != nil {
return fmt.Errorf("inspect Windows Job Object before detach: %w", err)
}
limits.BasicLimitInformation.LimitFlags &^= windows.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE
if _, err := windows.SetInformationJobObject(
c.job,
windows.JobObjectExtendedLimitInformation,
uintptr(unsafe.Pointer(&limits)),
uint32(unsafe.Sizeof(limits)),
); err != nil {
return fmt.Errorf("detach Windows Job Object: %w", err)
}
if err := windows.CloseHandle(c.job); err != nil {
c.job = 0
return fmt.Errorf("close detached Windows Job Object: %w", err)
}
c.job = 0
return nil
}

func (c *Controller) Close() error {
if c == nil {
return nil
Expand Down
32 changes: 32 additions & 0 deletions internal/process/process_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,38 @@ func TestWindowsJobObjectTerminatesAttachedProcess(t *testing.T) {
}
}

func TestWindowsJobObjectDetachKeepsAttachedProcessAlive(t *testing.T) {
cmd := exec.Command("powershell.exe", "-NoLogo", "-NoProfile", "-NonInteractive", "-Command", "Start-Sleep -Seconds 30")
if err := cmd.Start(); err != nil {
t.Fatal(err)
}
defer func() {
_ = cmd.Process.Kill()
_ = cmd.Wait()
}()

controller, err := Attach(cmd)
if err != nil {
t.Fatal(err)
}
if err := controller.Detach(); err != nil {
t.Fatal(err)
}

handle, err := windows.OpenProcess(windows.SYNCHRONIZE, false, uint32(cmd.Process.Pid))
if err != nil {
t.Fatalf("open detached process: %v", err)
}
defer windows.CloseHandle(handle)
state, err := windows.WaitForSingleObject(handle, 100)
if err != nil {
t.Fatalf("inspect detached process: %v", err)
}
if state != uint32(windows.WAIT_TIMEOUT) {
t.Fatalf("detached process exited unexpectedly, wait state=%d", state)
}
}

func TestWindowsJobObjectCloseTerminatesAttachedProcess(t *testing.T) {
cmd := exec.Command("powershell.exe", "-NoLogo", "-NoProfile", "-NonInteractive", "-Command", "Start-Sleep -Seconds 30")
if err := cmd.Start(); err != nil {
Expand Down
14 changes: 9 additions & 5 deletions scripts/install/install.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -1808,13 +1808,21 @@ exit `$LASTEXITCODE
$rollbackError = $null
$taskRecoveryPath = ''
try {
if ($effectivePrivilegeMode -eq 'elevated') {
# Stop the long-lived task owner before touching either stable shims or generation files.
Stop-ScheduledTask -TaskName 'AgentDock' -TaskPath '\' -ErrorAction SilentlyContinue
Start-Sleep -Milliseconds 500
}
if ($generationLayoutDetected -or $enginePrepared) {
# Target Core runs as agentdock-core.exe after both bootstrap and Update Engine.
# Stopping the CUI shim would miss the running generation and leave the new pointer live.
# Setup runtime hosts and elevated task hosts execute stable shims, so both layers
# must be quiesced before rollback restores the stable entry files.
$rollbackGenerationTray = Join-Path $generationBootstrapDirectory 'agentdock-tray.exe'
$rollbackGenerationCore = Join-Path $generationBootstrapDirectory 'agentdock-core.exe'
[void] (Stop-AgentDockTrayForUpgrade -BinaryPath $rollbackGenerationTray)
[void] (Stop-AgentDockForUpgrade -BinaryPath $rollbackGenerationCore)
[void] (Stop-AgentDockTrayForUpgrade -BinaryPath $destinationTrayBinary)
[void] (Stop-AgentDockForUpgrade -BinaryPath $destinationBinary)
} else {
if ($trayStopAttempted -or $stableFilesMayBeReplaced -or $trayStartupRegistrationChanged) {
[void] (Stop-AgentDockTrayForUpgrade -BinaryPath $destinationTrayBinary)
Expand All @@ -1823,10 +1831,6 @@ exit `$LASTEXITCODE
[void] (Stop-AgentDockForUpgrade -BinaryPath $destinationBinary)
}
}
if ($effectivePrivilegeMode -eq 'elevated') {
Stop-ScheduledTask -TaskName 'AgentDock' -TaskPath '\' -ErrorAction SilentlyContinue
Start-Sleep -Milliseconds 500
}

if ($stableFilesMayBeReplaced) {
$trayBackupExists = Test-Path -LiteralPath $trayBackup -PathType Leaf
Expand Down
37 changes: 34 additions & 3 deletions scripts/install/launch-windows-process.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ $settings = New-ScheduledTaskSettingsSet `
-DontStopIfGoingOnBatteries

$registered = $false
$startedAt = Get-Date
$launchSucceeded = $false
try {
Register-ScheduledTask `
-TaskName $taskName `
Expand All @@ -151,6 +151,9 @@ try {
-Settings $settings `
-Force | Out-Null
$registered = $true
# Task Scheduler timestamps can lag the caller's wall clock. This task name is
# unique, so compare the task's own before/after state instead of two clocks.
$initialLastRunTime = (Get-ScheduledTaskInfo -TaskName $taskName -TaskPath '\' -ErrorAction Stop).LastRunTime
& $AgentDockBinary service task-start `
--task-name $taskName `
--expected-user-sid $identity.User.Value | Out-Null
Expand All @@ -162,12 +165,13 @@ try {
do {
$task = Get-ScheduledTask -TaskName $taskName -TaskPath '\' -ErrorAction Stop
$info = Get-ScheduledTaskInfo -TaskName $taskName -TaskPath '\' -ErrorAction Stop
$hasRun = $info.LastRunTime -ge $startedAt.AddSeconds(-1)
$hasRun = $task.State -eq 'Running' -or $info.LastRunTime -ne $initialLastRunTime
if ($hasRun) {
if (-not $WaitForExit) {
if ($task.State -eq 'Ready' -and $info.LastTaskResult -ne 0) {
throw "Runtime process failed to launch, Task Scheduler result: $($info.LastTaskResult)."
}
$launchSucceeded = $true
return
}
if ($task.State -notin @('Running', 'Queued')) {
Expand All @@ -179,6 +183,7 @@ try {
-StdoutPath $stdoutPath `
-StderrPath $stderrPath)
}
$launchSucceeded = $true
return
}
}
Expand All @@ -191,9 +196,35 @@ try {
throw "Runtime process did not start within $TimeoutSeconds seconds."
} finally {
if ($registered) {
if (-not $launchSucceeded) {
# A failed wait must stop the unique Task action before unregistering it.
# The Go setup runtime host owns waited descendants through a kill-on-close Job.
Stop-ScheduledTask -TaskName $taskName -TaskPath '\' -ErrorAction SilentlyContinue
$stopDeadline = [DateTime]::UtcNow.AddSeconds(5)
do {
$remainingTask = Get-ScheduledTask -TaskName $taskName -TaskPath '\' -ErrorAction SilentlyContinue
if ($null -eq $remainingTask -or $remainingTask.State -notin @('Running', 'Queued')) {
break
}
Start-Sleep -Milliseconds 100
} while ([DateTime]::UtcNow -lt $stopDeadline)
}
Unregister-ScheduledTask -TaskName $taskName -TaskPath '\' -Confirm:$false -ErrorAction SilentlyContinue
}
if (-not [string]::IsNullOrWhiteSpace($diagnosticRoot)) {
Remove-Item -LiteralPath $diagnosticRoot -Recurse -Force -ErrorAction SilentlyContinue
# Task Scheduler can report Ready slightly before the terminated host releases
# inherited stdout/stderr handles. Keep cleanup bounded without rediscovering
# process trees in PowerShell.
$cleanupDeadline = [DateTime]::UtcNow.AddSeconds(5)
do {
Remove-Item -LiteralPath $diagnosticRoot -Recurse -Force -ErrorAction SilentlyContinue
if (-not (Test-Path -LiteralPath $diagnosticRoot)) {
break
}
Start-Sleep -Milliseconds 100
} while ([DateTime]::UtcNow -lt $cleanupDeadline)
if (Test-Path -LiteralPath $diagnosticRoot) {
Write-Warning "Unable to remove Setup runtime diagnostics directory: $diagnosticRoot"
}
}
}
13 changes: 11 additions & 2 deletions scripts/test/install_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,8 @@ func TestWindowsSetupLaunchesRuntimeOutsideRedirectionGuardTree(t *testing.T) {
"-LogonType Interactive",
"-RunLevel Limited",
"Register-ScheduledTask",
"$initialLastRunTime = (Get-ScheduledTaskInfo",
"$task.State -eq 'Running' -or $info.LastRunTime -ne $initialLastRunTime",
"& $AgentDockBinary service task-start",
"--task-name $taskName",
"--expected-user-sid $identity.User.Value",
Expand All @@ -1036,6 +1038,8 @@ func TestWindowsSetupLaunchesRuntimeOutsideRedirectionGuardTree(t *testing.T) {
"Task Scheduler result: $rawResult",
"Read-RuntimeDiagnosticTail",
"Remove-Item -LiteralPath $diagnosticRoot -Recurse -Force",
"Stop-ScheduledTask -TaskName $taskName",
"$launchSucceeded = $true",
"Unregister-ScheduledTask",
"AGENTDOCK_HOME",
"AGENTDOCK_DEFAULT_DIR",
Expand All @@ -1047,9 +1051,14 @@ func TestWindowsSetupLaunchesRuntimeOutsideRedirectionGuardTree(t *testing.T) {
if !strings.Contains(brokerScript, "finally {") || !strings.Contains(brokerScript, "Unregister-ScheduledTask") {
t.Fatal("runtime launch broker must remove its temporary task even when launch fails")
}
for _, forbidden := range []string{"-Execute $powerShellPath", "-EncodedCommand $encodedCommand"} {
for _, forbidden := range []string{
"-Execute $powerShellPath",
"-EncodedCommand $encodedCommand",
"$startedAt = Get-Date",
"Get-CimInstance Win32_Process",
} {
if strings.Contains(brokerScript, forbidden) {
t.Fatalf("runtime launch broker must not use a console-subsystem PowerShell task action: %q", forbidden)
t.Fatalf("runtime launch broker contains forbidden legacy launch behavior: %q", forbidden)
}
}

Expand Down
99 changes: 98 additions & 1 deletion scripts/test/test-windows-runtime-launch-diagnostics.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ param(
[string] $AgentDockBinary,
[Parameter(Mandatory = $true)]
[ValidateNotNullOrEmpty()]
[string] $HiddenHostBinary
[string] $HiddenHostBinary,
[ValidateRange(0, 3600)]
[int] $SchedulerClockSkewSeconds = 60
)

Set-StrictMode -Version Latest
Expand Down Expand Up @@ -51,6 +53,27 @@ $taskPrefix = 'AgentDock Setup Runtime '
$tempPrefix = 'agentdock-setup-runtime-'
$beforeTasks = @(Get-ScheduledTask -ErrorAction Stop | Where-Object { $_.TaskName.StartsWith($taskPrefix) } | ForEach-Object TaskName)
$beforeTempDirs = @(Get-ChildItem -LiteralPath ([IO.Path]::GetTempPath()) -Directory -Filter "$tempPrefix*" -ErrorAction SilentlyContinue | ForEach-Object FullName)
$realTaskInfoCommand = Get-Command Get-ScheduledTaskInfo
$realStopTaskCommand = Get-Command Stop-ScheduledTask
$stopRequests = New-Object 'System.Collections.Generic.List[string]'

function Get-ScheduledTaskInfo {
[CmdletBinding()]
param([string] $TaskName, [string] $TaskPath = '\')
$info = & $realTaskInfoCommand @PSBoundParameters
$lastRunTime = $info.LastRunTime
if ($lastRunTime.Year -ge 2000) {
$lastRunTime = $lastRunTime.AddSeconds(-$SchedulerClockSkewSeconds)
}
return [pscustomobject]@{ LastRunTime = $lastRunTime; LastTaskResult = $info.LastTaskResult }
}

function Stop-ScheduledTask {
[CmdletBinding()]
param([string] $TaskName, [string] $TaskPath = '\')
$stopRequests.Add($TaskName)
& $realStopTaskCommand @PSBoundParameters
}

try {
New-Item -ItemType Directory -Path $testRoot -Force | Out-Null
Expand Down Expand Up @@ -141,6 +164,80 @@ try {
throw "Detached runtime child unexpectedly owns a console window: $detachedState"
}

# Waited launchers can intentionally leave a long-lived descendant after the short command
# succeeds (service start does this for Core). Success must disarm kill-on-close ownership.
$successDescendantPidPath = Join-Path $testRoot 'wait-success-descendant-pid.txt'
$encodedSuccessDescendantPidPath = [Convert]::ToBase64String([Text.Encoding]::UTF8.GetBytes($successDescendantPidPath))
[IO.File]::WriteAllText(
$childScript,
"`$pidPath = [Text.Encoding]::UTF8.GetString([Convert]::FromBase64String('$encodedSuccessDescendantPidPath'))`r`n" +
"`$child = Start-Process -FilePath (Join-Path `$PSHOME 'powershell.exe') -ArgumentList '-NoLogo','-NoProfile','-NonInteractive','-Command','Start-Sleep -Seconds 20' -WindowStyle Hidden -PassThru`r`n" +
"[IO.File]::WriteAllText(`$pidPath, [string]`$child.Id)`r`nexit 0`r`n",
[Text.UTF8Encoding]::new($false)
)
& $resolvedLauncher `
-FilePath (Join-Path $PSHOME 'powershell.exe') `
-AgentDockBinary $resolvedAgentDockBinary `
-HiddenHostBinary $resolvedHiddenHostBinary `
-Arguments $arguments `
-WaitForExit `
-TimeoutSeconds 30

if (-not (Test-Path -LiteralPath $successDescendantPidPath -PathType Leaf)) {
throw 'Successful waited runtime launch did not publish its descendant PID.'
}
$successDescendantId = [int][IO.File]::ReadAllText($successDescendantPidPath)
$successDescendant = Get-Process -Id $successDescendantId -ErrorAction SilentlyContinue
if ($null -eq $successDescendant) {
throw 'Successful waited runtime launch killed the intentional long-lived descendant.'
}
Stop-Process -Id $successDescendantId -Force -ErrorAction SilentlyContinue

# A real wait timeout must stop its Task host. The host owns the waited process tree through
# a kill-on-close Job, so the child must be gone before rollback can touch stable binaries.
$stopsBeforeTimeout = $stopRequests.Count
$timeoutPidPath = Join-Path $testRoot 'timeout-child-pid.txt'
$encodedTimeoutPidPath = [Convert]::ToBase64String([Text.Encoding]::UTF8.GetBytes($timeoutPidPath))
[IO.File]::WriteAllText(
$childScript,
"`$pidPath = [Text.Encoding]::UTF8.GetString([Convert]::FromBase64String('$encodedTimeoutPidPath'))`r`n" +
"[IO.File]::WriteAllText(`$pidPath, [string]`$PID)`r`nStart-Sleep -Seconds 20`r`n",
[Text.UTF8Encoding]::new($false)
)
$timeoutMessage = ''
$timeoutChildSurvived = $false
try {
& $resolvedLauncher `
-FilePath (Join-Path $PSHOME 'powershell.exe') `
-AgentDockBinary $resolvedAgentDockBinary `
-HiddenHostBinary $resolvedHiddenHostBinary `
-Arguments $arguments `
-WaitForExit `
-TimeoutSeconds 2
} catch {
$timeoutMessage = $_.Exception.Message
} finally {
if (Test-Path -LiteralPath $timeoutPidPath -PathType Leaf) {
$timeoutChildId = [int][IO.File]::ReadAllText($timeoutPidPath)
$childStopDeadline = [DateTime]::UtcNow.AddSeconds(2)
while ($null -ne (Get-Process -Id $timeoutChildId -ErrorAction SilentlyContinue) -and
[DateTime]::UtcNow -lt $childStopDeadline) {
Start-Sleep -Milliseconds 100
}
$timeoutChildSurvived = $null -ne (Get-Process -Id $timeoutChildId -ErrorAction SilentlyContinue)
Stop-Process -Id $timeoutChildId -Force -ErrorAction SilentlyContinue
}
}
if (-not $timeoutMessage.Contains('Runtime process did not finish within 2 seconds.')) {
throw "Expected a bounded wait timeout, got: $timeoutMessage"
}
if ($stopRequests.Count -le $stopsBeforeTimeout) {
throw 'Timed-out runtime task was not stopped before unregistering.'
}
if ($timeoutChildSurvived) {
throw 'Timed-out wait-host child survived and could retain installer file handles.'
}

$afterTasks = @(Get-ScheduledTask -ErrorAction Stop | Where-Object { $_.TaskName.StartsWith($taskPrefix) } | ForEach-Object TaskName)
$newTasks = @($afterTasks | Where-Object { $_ -notin $beforeTasks })
if ($newTasks.Count -gt 0) {
Expand Down
Loading