Skip to content

Commit

Permalink
Don't use the wrong PID in error strings
Browse files Browse the repository at this point in the history
Instead of using the PID in the argument, the error strings were
constructed using the PID of the currently running process. This would
have caused a panic because the PID file cleanup is called before the
supervised process is started. So the s.cmd would have been nil.

Don't put the PID in the errors at this level at all, but wrap the error
once with the actual PID read from the PID file in maybeKillPidFile.

Signed-off-by: Tom Wieczorek <[email protected]>
(cherry picked from commit eb0839f)
  • Loading branch information
twz123 authored and github-actions[bot] committed Jun 4, 2024
1 parent f704a85 commit f76bc8d
Showing 1 changed file with 9 additions and 5 deletions.
14 changes: 9 additions & 5 deletions pkg/supervisor/supervisor_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ Loop:
if errors.Is(err, syscall.ESRCH) {
return nil
} else if err != nil {
return fmt.Errorf("failed to send SIGTERM to pid %d: %w", s.cmd.Process.Pid, err)
return fmt.Errorf("failed to send SIGTERM: %w", err)
}
case <-deadline:
break Loop
Expand All @@ -88,7 +88,7 @@ Loop:
if errors.Is(err, syscall.ESRCH) {
return nil
} else if err != nil {
return fmt.Errorf("failed to send SIGKILL to pid %d: %w", s.cmd.Process.Pid, err)
return fmt.Errorf("failed to send SIGKILL: %w", err)
}
return nil
}
Expand All @@ -115,15 +115,19 @@ func (s *Supervisor) maybeKillPidFile(check <-chan time.Time, deadline <-chan ti
return fmt.Errorf("failed to parse pid file %s: %w", s.PidFile, err)
}

return s.killPid(p, check, deadline)
if err := s.killPid(p, check, deadline); err != nil {
return fmt.Errorf("failed to kill process with PID %d: %w", p, err)
}

return nil
}

func (s *Supervisor) shouldKillProcess(pid int) (bool, error) {
cmdline, err := os.ReadFile(filepath.Join(s.ProcFSPath, strconv.Itoa(pid), "cmdline"))
if os.IsNotExist(err) {
return false, nil
} else if err != nil {
return false, fmt.Errorf("failed to read process %d cmdline: %w", pid, err)
return false, fmt.Errorf("failed to read process cmdline: %w", err)
}

// only kill process if it has the expected cmd
Expand All @@ -137,7 +141,7 @@ func (s *Supervisor) shouldKillProcess(pid int) (bool, error) {
if os.IsNotExist(err) {
return false, nil
} else if err != nil {
return false, fmt.Errorf("failed to read process %d environ: %w", pid, err)
return false, fmt.Errorf("failed to read process environ: %w", err)
}

for _, e := range strings.Split(string(env), "\x00") {
Expand Down

0 comments on commit f76bc8d

Please sign in to comment.