comby: kill child processes on context timeout
Created by: rvantonder
This is a cleaner follow up to #7177. See #7177 for details, but in summary:
CommandContext is used to kill comby when it takes too long, but only kills the parent comby process, and all child processes are oprhaned.
Setting
cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
and then killing the PID is the right thing to do to kill child processes. But doing so doesn't work withCommandContext
.
The solution takes care of manually removing child processes when ctx.Done
is reached:
- start the command (blocking, so that PID exists when we
select
on completion or context deadline as per https://github.com/sourcegraph/sourcegraph/pull/7177/files#r357532408) - wait for the command (nonblocking, in separate goroutine). This function is extracted as per https://github.com/sourcegraph/sourcegraph/pull/7177/files#r357532693
- select on
ctx.Done
or wait completion (blocking), and send sigkill to get rid of child processes
Tested manually, since it's hard to check that child processes are properly receiving sigkill
. I used this script that reports child proc status in a tight loop:
#!/bin/bash
while [ : ]; do
ps -aef | grep comby
sleep .1
done
and then ran a search command
repo:^github\.com/sourcegraph/sourcegraph$ "func Test" count:10000 lang:go timeout:500ms
multiple times. Before this PR, the orphaned child procs are (briefly) visible, and with the change, they are not.
Merge request reports
Activity
Created by: codecov[bot]
Codecov Report
Merging #7455 into master will increase coverage by
<.01%
. The diff coverage is44.44%
.@@ Coverage Diff @@ ## master #7455 +/- ## ========================================= + Coverage 40.09% 40.1% +<.01% ========================================= Files 1252 1252 Lines 65825 65845 +20 Branches 6165 6165 ========================================= + Hits 26395 26404 +9 - Misses 37029 37039 +10 - Partials 2401 2402 +1
Impacted Files Coverage Δ internal/comby/comby.go 55.04% <44.44%> (-2.26%)