From 5c12e3b3c6607fea648be8b445ed156403ff98a7 Mon Sep 17 00:00:00 2001 From: x x Date: Mon, 5 Oct 2026 20:58:07 +0800 Subject: [PATCH] =?UTF-8?q?fix(windows):=20=E6=94=B6=E6=95=9B=20Setup=20?= =?UTF-8?q?=E4=B8=B4=E6=97=B6=E4=BB=BB=E5=8A=A1=E7=94=9F=E5=91=BD=E5=91=A8?= =?UTF-8?q?=E6=9C=9F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../setup_runtime_host_windows.go | 24 ++++- internal/process/process_windows.go | 42 ++++++++ internal/process/process_windows_test.go | 32 ++++++ scripts/install/install.ps1 | 14 ++- scripts/install/launch-windows-process.ps1 | 37 ++++++- scripts/test/install_windows_test.go | 13 ++- ...est-windows-runtime-launch-diagnostics.ps1 | 99 ++++++++++++++++++- 7 files changed, 248 insertions(+), 13 deletions(-) diff --git a/cmd/agentdock-shim/setup_runtime_host_windows.go b/cmd/agentdock-shim/setup_runtime_host_windows.go index 4b17cac8..6ec8aee3 100644 --- a/cmd/agentdock-shim/setup_runtime_host_windows.go +++ b/cmd/agentdock-shim/setup_runtime_host_windows.go @@ -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) { @@ -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 } diff --git a/internal/process/process_windows.go b/internal/process/process_windows.go index e0172ebd..ed3b5a49 100644 --- a/internal/process/process_windows.go +++ b/internal/process/process_windows.go @@ -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 diff --git a/internal/process/process_windows_test.go b/internal/process/process_windows_test.go index 4fb87445..f3e80cce 100644 --- a/internal/process/process_windows_test.go +++ b/internal/process/process_windows_test.go @@ -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 { diff --git a/scripts/install/install.ps1 b/scripts/install/install.ps1 index a2b75c7e..16fcf573 100644 --- a/scripts/install/install.ps1 +++ b/scripts/install/install.ps1 @@ -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) @@ -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 diff --git a/scripts/install/launch-windows-process.ps1 b/scripts/install/launch-windows-process.ps1 index 52fdbd26..83622735 100644 --- a/scripts/install/launch-windows-process.ps1 +++ b/scripts/install/launch-windows-process.ps1 @@ -142,7 +142,7 @@ $settings = New-ScheduledTaskSettingsSet ` -DontStopIfGoingOnBatteries $registered = $false -$startedAt = Get-Date +$launchSucceeded = $false try { Register-ScheduledTask ` -TaskName $taskName ` @@ -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 @@ -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')) { @@ -179,6 +183,7 @@ try { -StdoutPath $stdoutPath ` -StderrPath $stderrPath) } + $launchSucceeded = $true return } } @@ -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" + } } } diff --git a/scripts/test/install_windows_test.go b/scripts/test/install_windows_test.go index 6fed3af5..fa35ff24 100644 --- a/scripts/test/install_windows_test.go +++ b/scripts/test/install_windows_test.go @@ -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", @@ -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", @@ -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) } } diff --git a/scripts/test/test-windows-runtime-launch-diagnostics.ps1 b/scripts/test/test-windows-runtime-launch-diagnostics.ps1 index 267474fd..2b4a5b99 100644 --- a/scripts/test/test-windows-runtime-launch-diagnostics.ps1 +++ b/scripts/test/test-windows-runtime-launch-diagnostics.ps1 @@ -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 @@ -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 @@ -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) {