mirror of
https://github.com/Cheviiot/Vintner.git
synced 2026-08-03 15:57:24 +00:00
Stop orphaning wine subprocesses when vintner is killed by PID
Every wrapped tool invocation (cl/link/msbuild/etc via wine, plus the native cmd/findstr shims) now starts its child in its own process group and forwards SIGINT/SIGTERM to that group, escalating to SIGKILL after a 5s grace period if it doesn't exit. Previously, interactive Ctrl-C happened to work by accident (the child inherited the terminal's foreground process group and got the signal directly), but anything that signals vintner by PID alone - a CI job's timeout, a supervisor's `kill <pid>` - never reached the wine/wineserver tree underneath it, which got reparented to init and kept running: wasted CPU, held file locks, stray FIFOs/temp files. Verified two ways: a unit test (signals_test.go) that starts a detached `sleep 30`, signals the test process itself, and checks the child actually dies; and a real end-to-end run - killed an in-flight `msbuild` driver build by PID mid-compile and confirmed no orphaned msbuild/cl/link/vintner process was left behind (wineserver and its persistent service processes are expected to survive, by design - see pipeDrainGrace's doc comment).
This commit is contained in:
@@ -29,7 +29,13 @@ func execInherit(args []string) int {
|
||||
cmd.Stdin = os.Stdin
|
||||
cmd.Stdout = os.Stdout
|
||||
cmd.Stderr = os.Stderr
|
||||
if err := cmd.Run(); err != nil {
|
||||
setNewProcessGroup(cmd)
|
||||
if err := cmd.Start(); err != nil {
|
||||
return 127
|
||||
}
|
||||
stopSignals := forwardSignals(cmd.Process)
|
||||
defer stopSignals()
|
||||
if err := cmd.Wait(); err != nil {
|
||||
if exitErr, ok := err.(*exec.ExitError); ok {
|
||||
return exitErr.ExitCode()
|
||||
}
|
||||
|
||||
@@ -91,6 +91,7 @@ func Run(tool string, args []string) int {
|
||||
}
|
||||
cmd.Env = env
|
||||
cmd.Stdin = os.Stdin
|
||||
setNewProcessGroup(cmd)
|
||||
exitCode = runRawStdout(cmd)
|
||||
default:
|
||||
relay := filepath.Join(paths.BaseUnix, "bin", toolRelayName)
|
||||
@@ -100,6 +101,7 @@ func Run(tool string, args []string) int {
|
||||
cmd := exec.Command(wineBin, append([]string{toolExePath}, rewritten...)...)
|
||||
cmd.Env = buildEnv(paths)
|
||||
cmd.Stdin = os.Stdin
|
||||
setNewProcessGroup(cmd)
|
||||
exitCode = runFiltered(cmd, s.stdoutFilter, s.stderrFilter)
|
||||
}
|
||||
}
|
||||
@@ -137,6 +139,7 @@ func runViaToolRelay(wineBin, relayExe, exePath string, args []string, paths *wi
|
||||
cmdArgs := append([]string{relayExe, exePath}, args...)
|
||||
cmd := exec.Command(wineBin, cmdArgs...)
|
||||
cmd.Env = append(buildEnv(paths), "MSVCGOWINE_STDOUT="+stdoutFifo, "MSVCGOWINE_STDERR="+stderrFifo)
|
||||
setNewProcessGroup(cmd)
|
||||
if devNull, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0); err == nil {
|
||||
defer devNull.Close()
|
||||
cmd.Stdout = devNull
|
||||
@@ -147,6 +150,8 @@ func runViaToolRelay(wineBin, relayExe, exePath string, args []string, paths *wi
|
||||
fmt.Fprintln(os.Stderr, "vintner:", err)
|
||||
return 1
|
||||
}
|
||||
stopSignals := forwardSignals(cmd.Process)
|
||||
defer stopSignals()
|
||||
|
||||
var wg sync.WaitGroup
|
||||
wg.Add(2)
|
||||
@@ -204,6 +209,8 @@ func runRawStdout(cmd *exec.Cmd) int {
|
||||
fmt.Fprintln(os.Stderr, "vintner:", err)
|
||||
return 1
|
||||
}
|
||||
stopSignals := forwardSignals(cmd.Process)
|
||||
defer stopSignals()
|
||||
|
||||
doneOut := make(chan struct{})
|
||||
doneErr := make(chan struct{})
|
||||
@@ -278,6 +285,8 @@ func runFiltered(cmd *exec.Cmd, stdoutF, stderrF lineFilter) int {
|
||||
fmt.Fprintln(os.Stderr, "vintner:", err)
|
||||
return 1
|
||||
}
|
||||
stopSignals := forwardSignals(cmd.Process)
|
||||
defer stopSignals()
|
||||
|
||||
doneOut := make(chan struct{})
|
||||
doneErr := make(chan struct{})
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
package wrapper
|
||||
|
||||
import (
|
||||
"os"
|
||||
"os/exec"
|
||||
"os/signal"
|
||||
"syscall"
|
||||
"time"
|
||||
)
|
||||
|
||||
// killGrace bounds how long a forwarded SIGINT/SIGTERM gets to make a
|
||||
// subprocess tree exit on its own before escalating to SIGKILL - long
|
||||
// enough for wineserver to tear down a Windows process tree cleanly, short
|
||||
// enough that an unresponsive one doesn't hang vintner's own shutdown.
|
||||
const killGrace = 5 * time.Second
|
||||
|
||||
// setNewProcessGroup puts cmd's eventual child in its own process group
|
||||
// (pgid = its own pid) instead of inheriting vintner's. Without this, a
|
||||
// caller that signals vintner by PID alone (a CI runner enforcing a
|
||||
// timeout, a supervisor's `kill <pid>`) never reaches the wine/wineserver
|
||||
// tree underneath it, which is then reparented to init and keeps running -
|
||||
// wasting CPU, holding file locks, leaving stray FIFOs/temp files behind.
|
||||
// (Interactive Ctrl-C already reaches every process in the terminal's
|
||||
// foreground group regardless of this, but forwardSignals below handles
|
||||
// that case too now that the child has moved to its own group.)
|
||||
func setNewProcessGroup(cmd *exec.Cmd) {
|
||||
cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
|
||||
}
|
||||
|
||||
// forwardSignals relays SIGINT/SIGTERM received by vintner itself to
|
||||
// proc's entire process group (proc must have been started via a cmd that
|
||||
// called setNewProcessGroup, making proc.Pid also the group id), escalating
|
||||
// to SIGKILL after killGrace if the group hasn't exited by then. Callers
|
||||
// must call the returned stop func once the process has actually exited
|
||||
// (e.g. right after cmd.Wait() returns), both to stop listening for
|
||||
// signals and to cancel a pending escalation.
|
||||
func forwardSignals(proc *os.Process) (stop func()) {
|
||||
sigCh := make(chan os.Signal, 1)
|
||||
signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM)
|
||||
done := make(chan struct{})
|
||||
|
||||
go func() {
|
||||
pgid := -proc.Pid
|
||||
for {
|
||||
select {
|
||||
case sig := <-sigCh:
|
||||
s, ok := sig.(syscall.Signal)
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
_ = syscall.Kill(pgid, s)
|
||||
select {
|
||||
case <-time.After(killGrace):
|
||||
_ = syscall.Kill(pgid, syscall.SIGKILL)
|
||||
case <-done:
|
||||
return
|
||||
}
|
||||
case <-done:
|
||||
return
|
||||
}
|
||||
}
|
||||
}()
|
||||
|
||||
return func() {
|
||||
signal.Stop(sigCh)
|
||||
close(done)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,43 @@
|
||||
package wrapper
|
||||
|
||||
import (
|
||||
"os"
|
||||
"os/exec"
|
||||
"syscall"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// TestForwardSignalsKillsChild verifies the actual mechanism that keeps a
|
||||
// wine subprocess from being orphaned: a SIGTERM delivered to the current
|
||||
// process (mimicking `kill <vintner-pid>`, not an interactive Ctrl-C) must
|
||||
// reach a child started with setNewProcessGroup, even though it's no longer
|
||||
// in the same process group.
|
||||
func TestForwardSignalsKillsChild(t *testing.T) {
|
||||
cmd := exec.Command("sleep", "30")
|
||||
setNewProcessGroup(cmd)
|
||||
if err := cmd.Start(); err != nil {
|
||||
t.Fatalf("starting sleep: %v", err)
|
||||
}
|
||||
stop := forwardSignals(cmd.Process)
|
||||
defer stop()
|
||||
|
||||
// signal.Notify (inside forwardSignals) intercepts this rather than
|
||||
// letting it terminate the test binary itself.
|
||||
if err := syscall.Kill(os.Getpid(), syscall.SIGTERM); err != nil {
|
||||
t.Fatalf("signaling self: %v", err)
|
||||
}
|
||||
|
||||
done := make(chan error, 1)
|
||||
go func() { done <- cmd.Wait() }()
|
||||
|
||||
select {
|
||||
case err := <-done:
|
||||
if err == nil {
|
||||
t.Fatal("expected the child to be killed by the forwarded signal, but it exited successfully")
|
||||
}
|
||||
case <-time.After(3 * time.Second):
|
||||
cmd.Process.Kill()
|
||||
t.Fatal("child was still running 3s after the signal should have been forwarded")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user