Skip to content

Improvements to Process.Kill() method. #31

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

Merged
merged 2 commits into from
Jun 4, 2024
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
5 changes: 3 additions & 2 deletions process.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
64 changes: 64 additions & 0 deletions process_linux.go
Original file line number Diff line number Diff line change
@@ -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)
}
14 changes: 12 additions & 2 deletions process_others.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
18 changes: 18 additions & 0 deletions process_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ package paths

import (
"context"
"runtime"
"testing"
"time"

Expand All @@ -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)
}
13 changes: 12 additions & 1 deletion process_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Loading