logmon: Fix a memory leak on task restart

Fix a logmon leak causing high goroutine and memory usage when a task
restarts.

Logmon `FileRotator` buffers the task stdout/stderr streams and
periodically flushing them to log files. Logmon creates a new
FileRotator for each stream for each task run. However, the
`flushPeriodically` goroutine is leaked when a task restarts,
holding a reference to a no-longer-needed `FileRotator` instance
along with its 64kb buffer.

The cause is that the code assumed `time.Ticker.Stop()` closes the
ticker channel, thereby terminating the goroutine, but the documentation
says otherwise:

> Stop turns off a ticker. After Stop, no more ticks will be sent. Stop does not close the channel, to prevent a concurrent goroutine reading from the channel from seeing an erroneous "tick".
https://pkg.go.dev/time#Ticker.Stop
This commit is contained in:
Mahmood Ali
2021-10-05 10:51:15 -04:00
parent 60a66a14b1
commit 6aa8913392

View File

@@ -71,7 +71,7 @@ func NewFileRotator(path string, baseFile string, maxFiles int,
flushTicker: time.NewTicker(bufferFlushDuration),
logger: logger,
purgeCh: make(chan struct{}, 1),
doneCh: make(chan struct{}, 1),
doneCh: make(chan struct{}),
}
if err := rotator.lastFile(); err != nil {
@@ -237,8 +237,14 @@ func (f *FileRotator) createFile() error {
// flushPeriodically flushes the buffered writer every 100ms to the underlying
// file
func (f *FileRotator) flushPeriodically() {
for range f.flushTicker.C {
f.flushBuffer()
for {
select {
case <-f.flushTicker.C:
f.flushBuffer()
case <-f.doneCh:
return
}
}
}
@@ -251,9 +257,9 @@ func (f *FileRotator) Close() error {
f.flushTicker.Stop()
f.flushBuffer()
// Stop the purge go routine
// Stop the go routines
if !f.closed {
f.doneCh <- struct{}{}
close(f.doneCh)
close(f.purgeCh)
f.closed = true
f.currentFile.Close()