Skip to content

comby: kill child processes on context timeout

Warren Gifford requested to merge rvt/destroy-child-processes into master

Created by: rvantonder

Preface: @keegancsmith and @slimsag have some existing context on this code, so I would appreciate a special eye from them.

CommandContext is used to kill comby when it takes too long, but only kills the parent comby process, and all child processes are oprhaned:

Screen Shot 2019-12-11 at 5 32 37 PM

Setting cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} and then killing the PID is the right thing to do. Unfortunately, my manual testing shows that CommandContext does not honor the cmd.SysProcAttr setting, and calling syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) manually instead successfully kills all processes.

Because it looks like I can't rely on CommandContext, I have to have some other way to read the context signal. This PR is an attempt and probably not correct because it just feels messy, but it's what I've come up with. It does "work" in successfully kill the child processes. So now, instead of using CommandContext:

  • run cmd.Start() and all subsequent logic in a separate goroutine
  • send err messages to a errMsgChannel
  • send struct{}{} to indicate successful completion to a successChannel
  • select on the channels, including context Done signal, to figure out whether the command completed successfully, unsuccessfully, or timed out.

Merge request reports

Loading