diff options
author | OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com> | 2022-07-21 14:55:31 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-07-21 14:55:31 +0200 |
commit | 53dfc23e429cd53124a54eb3f3066a67aa5ead4c (patch) | |
tree | c45f7f29de84d4bf442e607deb6520ce3e878008 | |
parent | 5abb38238d56f87e6fe277939b0fcccb78bcda42 (diff) | |
parent | 4e72aa58604f9384c3d2913b16baa41f00d0a9e1 (diff) | |
download | podman-53dfc23e429cd53124a54eb3f3066a67aa5ead4c.tar.gz podman-53dfc23e429cd53124a54eb3f3066a67aa5ead4c.tar.bz2 podman-53dfc23e429cd53124a54eb3f3066a67aa5ead4c.zip |
Merge pull request #14984 from Luap99/logs
fix goroutine leaks in events and logs backend
-rw-r--r-- | libpod/container_log.go | 14 | ||||
-rw-r--r-- | libpod/container_log_linux.go | 9 | ||||
-rw-r--r-- | libpod/events/journal_linux.go | 13 | ||||
-rw-r--r-- | libpod/events/logfile.go | 26 |
4 files changed, 38 insertions, 24 deletions
diff --git a/libpod/container_log.go b/libpod/container_log.go index a9e0fe065..c49b54eb1 100644 --- a/libpod/container_log.go +++ b/libpod/container_log.go @@ -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" ) @@ -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 { diff --git a/libpod/container_log_linux.go b/libpod/container_log_linux.go index 0686caed2..7e95f2449 100644 --- a/libpod/container_log_linux.go +++ b/libpod/container_log_linux.go @@ -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 diff --git a/libpod/events/journal_linux.go b/libpod/events/journal_linux.go index 0a0a768d0..16ef6504f 100644 --- a/libpod/events/journal_linux.go +++ b/libpod/events/journal_linux.go @@ -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 diff --git a/libpod/events/logfile.go b/libpod/events/logfile.go index 4dafd8600..c7dbf4850 100644 --- a/libpod/events/logfile.go +++ b/libpod/events/logfile.go @@ -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 } @@ -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 |