* fix: Remove double-kill bug in Windows process termination (#777) Problems fixed: - Removed double TASKKILL execution when SendInterrupt is enabled - Simplified kill logic to match Linux behavior (single kill command) - Added console window hiding for cleaner UX (no flashing windows) - Switched from PowerShell to cmd.exe for better performance - Improved logging to match Linux output format - Better error handling for already-terminated processes The previous code would run TASKKILL twice when SendInterrupt was enabled, or handle it inconsistently when disabled. This caused processes to not be properly terminated, leading to port conflicts and orphaned processes as reported in #777. Testing on Linux shows clean process transitions with no port conflicts. Windows users requested to test and verify the fix. Fixes #777 * refactor: Address review feedback - match Linux logging style - Use mainDebug instead of runnerLog to match Linux implementation - Remove verbose logging as requested by maintainer - Clarify why send_interrupt is not supported on Windows - Simplify error handling to match Linux style - Keep core fix: single TASKKILL, console hiding, proper Wait() Addresses feedback from @xiantang in PR review.
This commit is contained in:
+41
-16
@@ -1,45 +1,69 @@
|
||||
//go:build windows
|
||||
|
||||
package runner
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"os/exec"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
"syscall"
|
||||
)
|
||||
|
||||
func (e *Engine) killCmd(cmd *exec.Cmd) (pid int, err error) {
|
||||
pid = cmd.Process.Pid
|
||||
// https://stackoverflow.com/a/44551450
|
||||
kill := exec.Command("TASKKILL", "/T", "/F", "/PID", strconv.Itoa(pid))
|
||||
|
||||
// On Windows, SIGINT is not supported for process trees.
|
||||
// Windows uses different process termination mechanisms than Unix.
|
||||
// TASKKILL is the proper way to terminate process hierarchies on Windows.
|
||||
if e.config.Build.SendInterrupt {
|
||||
if err = kill.Run(); err != nil {
|
||||
return
|
||||
}
|
||||
time.Sleep(e.config.killDelay())
|
||||
e.mainLog("send_interrupt is not supported on Windows, using TASKKILL instead")
|
||||
}
|
||||
err = kill.Run()
|
||||
// Wait releases any resources associated with the Process.
|
||||
|
||||
// Use TASKKILL to forcefully terminate the entire process tree
|
||||
e.mainDebug("sending TASKKILL to process tree")
|
||||
killCmd := exec.Command("TASKKILL", "/F", "/T", "/PID", strconv.Itoa(pid))
|
||||
|
||||
// Hide the console window for cleaner UX
|
||||
killCmd.SysProcAttr = &syscall.SysProcAttr{
|
||||
HideWindow: true,
|
||||
CreationFlags: 0x08000000, // CREATE_NO_WINDOW
|
||||
}
|
||||
|
||||
err = killCmd.Run()
|
||||
|
||||
// Wait for the process to fully terminate and release resources
|
||||
_, _ = cmd.Process.Wait()
|
||||
|
||||
return pid, err
|
||||
}
|
||||
|
||||
func (e *Engine) startCmd(cmd string) (*exec.Cmd, io.ReadCloser, io.ReadCloser, error) {
|
||||
var err error
|
||||
|
||||
if !strings.Contains(cmd, ".exe") {
|
||||
e.runnerLog("CMD will not recognize non .exe file for execution, path: %s", cmd)
|
||||
if !strings.Contains(cmd, ".exe") && !strings.Contains(cmd, ".bat") && !strings.Contains(cmd, ".cmd") {
|
||||
e.mainDebug("command may not be recognized as executable: %s", cmd)
|
||||
}
|
||||
c := exec.Command("powershell", cmd)
|
||||
|
||||
// Use cmd.exe instead of PowerShell for better performance
|
||||
c := exec.Command("cmd", "/C", cmd)
|
||||
|
||||
// Hide the console window
|
||||
c.SysProcAttr = &syscall.SysProcAttr{
|
||||
HideWindow: true,
|
||||
CreationFlags: 0x08000000, // CREATE_NO_WINDOW
|
||||
}
|
||||
|
||||
stderr, err := c.StderrPipe()
|
||||
if err != nil {
|
||||
return nil, nil, nil, err
|
||||
return nil, nil, nil, fmt.Errorf("failed to create stderr pipe: %w", err)
|
||||
}
|
||||
|
||||
stdout, err := c.StdoutPipe()
|
||||
if err != nil {
|
||||
return nil, nil, nil, err
|
||||
return nil, nil, nil, fmt.Errorf("failed to create stdout pipe: %w", err)
|
||||
}
|
||||
|
||||
c.Stdout = os.Stdout
|
||||
@@ -47,7 +71,8 @@ func (e *Engine) startCmd(cmd string) (*exec.Cmd, io.ReadCloser, io.ReadCloser,
|
||||
|
||||
err = c.Start()
|
||||
if err != nil {
|
||||
return nil, nil, nil, err
|
||||
return nil, nil, nil, fmt.Errorf("failed to start command: %w", err)
|
||||
}
|
||||
return c, stdout, stderr, err
|
||||
|
||||
return c, stdout, stderr, nil
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user