Skip to content
Snippets Groups Projects

comby: kill child processes on context timeout

Merged Administrator requested to merge rvt/clean-child-process-cleanup into master

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 with CommandContext.

The solution takes care of manually removing child processes when ctx.Done is reached:


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

Merged by avatar (Jul 25, 2025 10:19pm UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Created by: codecov[bot]

    Codecov Report

    Merging #7455 into master will increase coverage by <.01%. The diff coverage is 44.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%) :arrow_down:
Please register or sign in to reply
Loading