diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-05-18 09:50:02 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-05-18 10:08:19 +0300 |
commit | faac47edca44039bcfcc4becb3be5b21b8dc5f95 (patch) | |
tree | 2da4c95e9aef151274f9cda818714224075a0790 | |
parent | 32474ae99c506421a9f681c704a35e63b47c8d2b (diff) |
supervisor: Fix leaking logrus Goroutine on spawn failurepks-supervise-fix-logrus-goroutine-leakage
For a long time we have observed leaking Goroutines in our supervisor
test which verifies that the circuit breaker kicks in in case spawning
of the process fails. As it turns out, this is a real issue: we set both
stdout and stderr of the new command to an `io.PipeWriter` created for
logging purposes. And because the Go runtime doesn't automatically close
these writers for us, we must make sure to do so ourselves. And while we
do that in case the command has been spawned successfully, we don't
handle the case where spawning has failed and thus don't close it at all
here.
Fix this Goroutine leakage by closing the `io.PipeWriter` in case we
fail to start the command.
Changelog: fixed
-rw-r--r-- | internal/supervisor/supervisor.go | 9 |
1 files changed, 8 insertions, 1 deletions
diff --git a/internal/supervisor/supervisor.go b/internal/supervisor/supervisor.go index 0c7310817..602010483 100644 --- a/internal/supervisor/supervisor.go +++ b/internal/supervisor/supervisor.go @@ -101,7 +101,14 @@ func (p *Process) start(logger *log.Entry) (*exec.Cmd, error) { cmd.Stdout = logWriter cmd.Stderr = logWriter - return cmd, cmd.Start() + // When starting the command fails we need to make sure to close the log pipes, or + // otherwise the Goroutines spawned by logrus may leak. + if err := cmd.Start(); err != nil { + logWriter.Close() + return nil, err + } + + return cmd, nil } func (p *Process) notifyEvent(eventType EventType, pid int) { |