I have a kind-of workaround.  Firstly, I see that Go has the ability to 
start a new session for the child with Setsid called.  However on timeout I 
still need to kill the process group (-pid) instead of the process, which I 
can do by implementing the context deadline manually:

        cmd := exec.Command("/bin/sh", "-c", "exec yes >/dev/null")
        if cmd.SysProcAttr == nil {
                cmd.SysProcAttr = &syscall.SysProcAttr{}
        }
        cmd.SysProcAttr.Setsid = true

        go func() {
                select {
                case <-ctx.Done():
                        // Race by doing it here and not in os/exec; maybe 
some other
                        // process gets the pid in the mean time
                        if cmd.Process.Pid > 1 && (cmd.ProcessState == nil 
|| !cmd.ProcessState.Exited()) {
                                err := syscall.Kill(-cmd.Process.Pid, 
os.Kill.(syscall.Signal))
                                if err != nil && err != syscall.ESRCH {
                                        fmt.Printf("Ooops, signal failed: 
%v\n", err)
                                }
                        }
                }
        }()

        err := cmd.Run()
        // rest of code the same

This appears to solve both problems: I get sensible resource accounting, 
*and* the shell and its children are killed.

1 Run(): signal: killed
rusage 1: Utime={0 271899}, Stime={0 727730}, Maxrss=1892
2 Run(): signal: killed
rusage 2: Utime={0 459969}, Stime={1 539899}, Maxrss=2004
4 Run(): signal: killed
rusage 4: Utime={0 979834}, Stime={3 19489}, Maxrss=1904
Bye!

To make this work without the race, I would like to suggest that the 
Process.Kill() <https://golang.org/pkg/os/#Process.Kill> function checks 
the value of SysProcAttr.Setsid, and if it's true, sends the kill signal to 
the process group rather than just the process.  This way, the regular 
exec.CommandContext would clean up properly.

(Note: I would *not* change Process.Signal(); any signal sent that way, 
including the Kill signal, should just go to the single process).

This is a nominally a change in behaviour, although I can't see that the 
current behaviour is actually very useful - that is, killing the parent, 
but letting its children re-attach themselves to pid 1 as (in effect) 
unsupervised daemons.  In addition, it would only be a change in behaviour 
for those people who have applied Setsid=true.

However if necessary it could be made backwards-compatible by adding a new 
flag to SysProcAttr, e.g. "KillProcessGroup" which would make Process.Kill 
send to -pid instead of pid.

Thoughts?

-- 
You received this message because you are subscribed to the Google Groups 
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to golang-nuts+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/golang-nuts/b471570e-8a69-44fc-8936-4698405487efo%40googlegroups.com.

Reply via email to