From fb2bb3653e2755d971f21debfecbd7c878a3c23f Mon Sep 17 00:00:00 2001 From: Kenfe-Mickael Laventure Date: Tue, 17 Jan 2017 13:12:50 -0800 Subject: [PATCH] Close logwatcher on context cancellation This commit addresses 2 issues: 1. in `tailfile()` if somehow the `logWatcher.Msg` were to become full and the watcher closed before space was made into it, we were getting stuck there forever since we were not checking for the watcher getting closed 2. when servicing `docker logs`, if the command was cancelled we were not closing the watcher (and hence notifying it to stop copying data) Signed-off-by: Kenfe-Mickael Laventure --- daemon/logger/jsonfilelog/read.go | 6 +++++- daemon/logs.go | 22 +++++++++++++--------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/daemon/logger/jsonfilelog/read.go b/daemon/logger/jsonfilelog/read.go index f2f9df1887..deff86dfa3 100644 --- a/daemon/logger/jsonfilelog/read.go +++ b/daemon/logger/jsonfilelog/read.go @@ -137,7 +137,11 @@ func tailFile(f io.ReadSeeker, logWatcher *logger.LogWatcher, tail int, since ti if !since.IsZero() && msg.Timestamp.Before(since) { continue } - logWatcher.Msg <- msg + select { + case <-logWatcher.WatchClose(): + return + case logWatcher.Msg <- msg: + } } } diff --git a/daemon/logs.go b/daemon/logs.go index 7bb5ee9e1f..b6c30f40ed 100644 --- a/daemon/logs.go +++ b/daemon/logs.go @@ -64,6 +64,18 @@ func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, c Follow: follow, } logs := logReader.ReadLogs(readConfig) + // Close logWatcher on exit + defer func() { + logs.Close() + if cLog != container.LogDriver { + // Since the logger isn't cached in the container, which + // occurs if it is running, it must get explicitly closed + // here to avoid leaking it and any file handles it has. + if err := cLog.Close(); err != nil { + logrus.Errorf("Error closing logger: %v", err) + } + } + }() wf := ioutils.NewWriteFlusher(config.OutStream) defer wf.Close() @@ -84,19 +96,11 @@ func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, c logrus.Errorf("Error streaming logs: %v", err) return nil case <-ctx.Done(): - logs.Close() + logrus.Debugf("logs: end stream, ctx is done: %v", ctx.Err()) return nil case msg, ok := <-logs.Msg: if !ok { logrus.Debug("logs: end stream") - logs.Close() - if cLog != container.LogDriver { - // Since the logger isn't cached in the container, which occurs if it is running, it - // must get explicitly closed here to avoid leaking it and any file handles it has. - if err := cLog.Close(); err != nil { - logrus.Errorf("Error closing logger: %v", err) - } - } return nil } logLine := msg.Line