Skip to content

Commit

Permalink
Merge pull request #14984 from Luap99/logs
Browse files Browse the repository at this point in the history
fix goroutine leaks in events and logs backend
  • Loading branch information
openshift-merge-robot authored Jul 21, 2022
2 parents 5abb382 + 4e72aa5 commit 53dfc23
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 24 deletions.
14 changes: 10 additions & 4 deletions libpod/container_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/containers/podman/v4/libpod/define"
"github.com/containers/podman/v4/libpod/events"
"github.com/containers/podman/v4/libpod/logs"
"github.com/nxadm/tail"
"github.com/nxadm/tail/watch"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -74,14 +75,19 @@ func (c *Container) readFromLogFile(ctx context.Context, options *logs.LogOption

go func() {
defer options.WaitGroup.Done()

for line := range t.Lines {
var line *tail.Line
var ok bool
for {
select {
case <-ctx.Done():
// the consumer has cancelled
t.Kill(errors.New("hangup by client"))
return
default:
// fallthrough
case line, ok = <-t.Lines:
if !ok {
// channel was closed
return
}
}
nll, err := logs.NewLogLine(line.Text)
if err != nil {
Expand Down
9 changes: 7 additions & 2 deletions libpod/container_log_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,13 @@ func (c *Container) readFromJournal(ctx context.Context, options *logs.LogOption
if !options.Follow || !containerCouldBeLogging {
return
}
// Sleep until something's happening on the journal.
journal.Wait(sdjournal.IndefiniteWait)

// journal.Wait() is blocking, this would cause the goroutine to hang forever
// if no more journal entries are generated and thus if the client
// has closed the connection in the meantime to leak memory.
// Waiting only 5 seconds makes sure we can check if the client closed in the
// meantime at least every 5 seconds.
journal.Wait(5 * time.Second)
continue
}
lastReadCursor = cursor
Expand Down
13 changes: 11 additions & 2 deletions libpod/events/journal_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,18 @@ func (e EventJournalD) Read(ctx context.Context, options ReadOptions) error {
if !options.Stream || (len(options.Until) > 0 && time.Now().After(untilTime)) {
break
}
t := sdjournal.IndefiniteWait

// j.Wait() is blocking, this would cause the goroutine to hang forever
// if no more journal entries are generated and thus if the client
// has closed the connection in the meantime to leak memory.
// Waiting only 5 seconds makes sure we can check if the client closed in the
// meantime at least every 5 seconds.
t := 5 * time.Second
if len(options.Until) > 0 {
t = time.Until(untilTime)
until := time.Until(untilTime)
if until < t {
t = until
}
}
_ = j.Wait(t)
continue
Expand Down
26 changes: 10 additions & 16 deletions libpod/events/logfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,23 +108,19 @@ func (e EventLogFile) Read(ctx context.Context, options ReadOptions) error {
}
}()
}
funcDone := make(chan bool)
copy := true
go func() {
select {
case <-funcDone:
// Do nothing
case <-ctx.Done():
copy = false
t.Kill(errors.New("hangup by client"))
}
}()
for line := range t.Lines {
var line *tail.Line
var ok bool
for {
select {
case <-ctx.Done():
// the consumer has cancelled
t.Kill(errors.New("hangup by client"))
return nil
default:
case line, ok = <-t.Lines:
if !ok {
// channel was closed
return nil
}
// fallthrough
}

Expand All @@ -138,12 +134,10 @@ func (e EventLogFile) Read(ctx context.Context, options ReadOptions) error {
default:
return fmt.Errorf("event type %s is not valid in %s", event.Type.String(), e.options.LogFilePath)
}
if copy && applyFilters(event, filterMap) {
if applyFilters(event, filterMap) {
options.EventChannel <- event
}
}
funcDone <- true
return nil
}

// String returns a string representation of the logger
Expand Down

0 comments on commit 53dfc23

Please sign in to comment.