From d7db0c861255a0787c88d38ded9a5cc35985baff Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 4 Jun 2024 11:29:26 +0200 Subject: [PATCH 1/2] Added test --- process_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/process_test.go b/process_test.go index 5346dda..e78ad2b 100644 --- a/process_test.go +++ b/process_test.go @@ -31,6 +31,7 @@ package paths import ( "context" + "runtime" "testing" "time" @@ -54,3 +55,20 @@ func TestProcessWithinContext(t *testing.T) { require.Less(t, time.Since(start), 500*time.Millisecond) cancel() } + +func TestKillProcessGroupOnLinux(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("skipping test on non-linux system") + } + + p, err := NewProcess(nil, "bash", "-c", "sleep 5 ; echo -n 5") + require.NoError(t, err) + start := time.Now() + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + _, _, err = p.RunAndCaptureOutput(ctx) + require.EqualError(t, err, "signal: killed") + // Assert that the process was killed within the timeout + require.Less(t, time.Since(start), 2*time.Second) +} From 2288ccbef928beef29f623c54ddd9a2fdf6c9420 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 4 Jun 2024 11:29:37 +0200 Subject: [PATCH 2/2] Process.Kill() on Linux now terminates the process group The kill method now kills also the children processes. --- process.go | 5 ++-- process_linux.go | 64 ++++++++++++++++++++++++++++++++++++++++++++++ process_others.go | 14 ++++++++-- process_windows.go | 13 +++++++++- 4 files changed, 91 insertions(+), 5 deletions(-) create mode 100644 process_linux.go diff --git a/process.go b/process.go index 4c86928..ebfe713 100644 --- a/process.go +++ b/process.go @@ -55,7 +55,8 @@ func NewProcess(extraEnv []string, args ...string) (*Process, error) { cmd: exec.Command(args[0], args[1:]...), } p.cmd.Env = append(os.Environ(), extraEnv...) - p.TellCommandNotToSpawnShell() + tellCommandNotToSpawnShell(p.cmd) // windows specific + tellCommandToStartOnNewProcessGroup(p.cmd) // linux specific // This is required because some tools detects if the program is running // from terminal by looking at the stdin/out bindings. @@ -146,7 +147,7 @@ func (p *Process) Signal(sig os.Signal) error { // actually exited. This only kills the Process itself, not any other processes it may // have started. func (p *Process) Kill() error { - return p.cmd.Process.Kill() + return kill(p.cmd) } // SetDir sets the working directory of the command. If Dir is the empty string, Run diff --git a/process_linux.go b/process_linux.go new file mode 100644 index 0000000..5735a85 --- /dev/null +++ b/process_linux.go @@ -0,0 +1,64 @@ +// +// This file is part of PathsHelper library. +// +// Copyright 2023 Arduino AG (http://www.arduino.cc/) +// +// PathsHelper library is free software; you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation; either version 2 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +// +// As a special exception, you may use this file as part of a free software +// library without restriction. Specifically, if other files instantiate +// templates or use macros or inline functions from this file, or you compile +// this file and link it with other files to produce an executable, this +// file does not by itself cause the resulting executable to be covered by +// the GNU General Public License. This exception does not however +// invalidate any other reasons why the executable file might be covered by +// the GNU General Public License. +// + +//go:build !windows + +package paths + +import ( + "os/exec" + "syscall" +) + +func tellCommandNotToSpawnShell(_ *exec.Cmd) { + // no op +} + +func tellCommandToStartOnNewProcessGroup(oscmd *exec.Cmd) { + // https://groups.google.com/g/golang-nuts/c/XoQ3RhFBJl8 + + // Start the process in a new process group. + // This is needed to kill the process and its children + // if we need to kill the process. + if oscmd.SysProcAttr == nil { + oscmd.SysProcAttr = &syscall.SysProcAttr{} + } + oscmd.SysProcAttr.Setpgid = true +} + +func kill(oscmd *exec.Cmd) error { + // https://groups.google.com/g/golang-nuts/c/XoQ3RhFBJl8 + + // Kill the process group + pgid, err := syscall.Getpgid(oscmd.Process.Pid) + if err != nil { + return err + } + return syscall.Kill(-pgid, syscall.SIGKILL) +} diff --git a/process_others.go b/process_others.go index 39bd3e1..70b6236 100644 --- a/process_others.go +++ b/process_others.go @@ -27,12 +27,22 @@ // the GNU General Public License. // -//go:build !windows +//go:build !windows && !linux package paths -import "os/exec" +import ( + "os/exec" +) func tellCommandNotToSpawnShell(_ *exec.Cmd) { // no op } + +func tellCommandToStartOnNewProcessGroup(_ *exec.Cmd) { + // no op +} + +func kill(oscmd *exec.Cmd) error { + return oscmd.Process.Kill() +} diff --git a/process_windows.go b/process_windows.go index 0c968ae..00cd6a3 100644 --- a/process_windows.go +++ b/process_windows.go @@ -35,5 +35,16 @@ import ( ) func tellCommandNotToSpawnShell(oscmd *exec.Cmd) { - oscmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true} + if oscmd.SysProcAttr == nil { + oscmd.SysProcAttr = &syscall.SysProcAttr{} + } + oscmd.SysProcAttr.HideWindow = true +} + +func tellCommandToStartOnNewProcessGroup(_ *exec.Cmd) { + // no op +} + +func kill(oscmd *exec.Cmd) error { + return oscmd.Process.Kill() }