Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use windows console groups for process management #879

Merged
merged 2 commits into from
Jan 3, 2019
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
4 changes: 2 additions & 2 deletions process/process_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,9 @@ func TestProcessCapturesOutputLineByLine(t *testing.T) {

func TestProcessInterrupts(t *testing.T) {
if runtime.GOOS == `windows` {
t.Skip("Not supported on windows")
t.Skip("Works in windows, but not in docker")
}

var lines []string
var mu sync.Mutex

Expand Down
46 changes: 30 additions & 16 deletions process/utils_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,39 +5,53 @@ import (
"os"
"os/exec"
"strconv"
"syscall"

"github.com/buildkite/agent/logger"
)

// Windows signal handling isn't a thing unfortunately, so we are limited in what we
// can do with gracefully terminating. Windows also doesn't have process hierarchies
// in the same way that unix does, so killing a process doesn't have any effect on
// the processes it spawned. We use TASKKILL.EXE with the /T flag to traverse processes
// that have a matching ParentProcessID, but even then if a process in the middle
// of the chain has died, we leave a heap of processes hanging around. Future improvements
// might include using terminal groups or job groups, which are a modern windows thing
// that might work for us.
// Windows has no concept of parent/child processes or signals. The best we can do
// is create processes inside a "console group" and then send break / ctrl-c events
// to that group. This is superior to walking a process tree to kill each process
// because that relies on each process in that chain still being active.

// See https://docs.microsoft.com/en-us/windows/console/generateconsolectrlevent

var (
libkernel32 = syscall.MustLoadDLL("kernel32")
procGenerateConsoleCtrlEvent = libkernel32.MustFindProc("GenerateConsoleCtrlEvent")
)

const (
createNewProcessGroupFlag = 0x00000200
)

func StartPTY(c *exec.Cmd) (*os.File, error) {
return nil, errors.New("PTY is not supported on Windows")
}

func createCommand(name string, arg ...string) *exec.Cmd {
return exec.Command(name, arg...)
func createCommand(cmd string, args ...string) *exec.Cmd {
execCmd := exec.Command(cmd, args...)
execCmd.SysProcAttr = &syscall.SysProcAttr{
CreationFlags: syscall.CREATE_UNICODE_ENVIRONMENT | createNewProcessGroupFlag,
}
return execCmd
}

func terminateProcess(p *os.Process, l *logger.Logger) error {
l.Debug("[Process] Terminating process tree with TASKKILL.EXE PID: %d", p.Pid)

// taskkill.exe with /F will call TerminateProcess and hard-kill the process and
// anything in it's process tree.
// anything left in it's process tree.
return exec.Command("CMD", "/C", "TASKKILL.EXE", "/F", "/T", "/PID", strconv.Itoa(p.Pid)).Run()
}

func interruptProcess(p *os.Process, l *logger.Logger) error {
l.Debug("[Process] Interrupting process tree with TASKKILL.EXE PID: %d", p.Pid)

// taskkill.exe without the /F will use window signalling (WM_STOP, WM_QUIT) to kind
// of gracefull shutdown windows things that support it.
return exec.Command("CMD", "/C", "TASKKILL.EXE", "/T", "/PID", strconv.Itoa(p.Pid)).Run()
// Sends a CTRL-BREAK signal to the process group id, which is the same as the process PID
// For some reason I cannot fathom, this returns "Incorrect function" in docker for windows
r1, _, err := procGenerateConsoleCtrlEvent.Call(syscall.CTRL_BREAK_EVENT, uintptr(p.Pid))
if r1 == 0 {
return err
}
return nil
}