From 68e71aa3e686b05987f139a7f64df304461b504a Mon Sep 17 00:00:00 2001
From: Jim Minter <jminter@redhat.com>
Date: Thu, 20 Apr 2017 11:17:06 +0100
Subject: [PATCH] Close logger only after StartLogger call

Signed-off-by: Jim Minter <jminter@redhat.com>
---
 daemon/attach.go | 10 +++++++++-
 daemon/logs.go   | 42 ++++++++++++++++++++----------------------
 2 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/daemon/attach.go b/daemon/attach.go
index a0e2b700ae..0a6c05dc2c 100644
--- a/daemon/attach.go
+++ b/daemon/attach.go
@@ -102,15 +102,23 @@ func (daemon *Daemon) ContainerAttachRaw(prefixOrName string, stdin io.ReadClose
 
 func (daemon *Daemon) containerAttach(c *container.Container, cfg *stream.AttachConfig, logs, doStream bool) error {
 	if logs {
-		logDriver, err := daemon.getLogger(c)
+		logDriver, logCreated, err := daemon.getLogger(c)
 		if err != nil {
 			return err
 		}
+		if logCreated {
+			defer func() {
+				if err = logDriver.Close(); err != nil {
+					logrus.Errorf("Error closing logger: %v", err)
+				}
+			}()
+		}
 		cLog, ok := logDriver.(logger.LogReader)
 		if !ok {
 			return logger.ErrReadLogsNotSupported
 		}
 		logs := cLog.ReadLogs(logger.ReadConfig{Tail: -1})
+		defer logs.Close()
 
 	LogLoop:
 		for {
diff --git a/daemon/logs.go b/daemon/logs.go
index b207fb693e..3d59fb3e72 100644
--- a/daemon/logs.go
+++ b/daemon/logs.go
@@ -45,10 +45,17 @@ func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, c
 		return nil, logger.ErrReadLogsNotSupported
 	}
 
-	cLog, err := daemon.getLogger(container)
+	cLog, cLogCreated, err := daemon.getLogger(container)
 	if err != nil {
 		return nil, err
 	}
+	if cLogCreated {
+		defer func() {
+			if err = cLog.Close(); err != nil {
+				logrus.Errorf("Error closing logger: %v", err)
+			}
+		}()
+	}
 
 	logReader, ok := cLog.(logger.LogReader)
 	if !ok {
@@ -85,23 +92,8 @@ func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, c
 	messageChan := make(chan *backend.LogMessage, 1)
 	go func() {
 		// set up some defers
-		defer func() {
-			// ok so this function, originally, was placed right after that
-			// logger.ReadLogs call above. I THINK that means it sets off the
-			// chain of events that results in the logger needing to be closed.
-			// i do not know if an error in time parsing above causing an early
-			// return will result in leaking the logger. if that is the case,
-			// it would also have been a bug in the original code
-			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)
-				}
-			}
-		}()
+		defer logs.Close()
+
 		// close the messages channel. closing is the only way to signal above
 		// that we're doing with logs (other than context cancel i guess).
 		defer close(messageChan)
@@ -148,11 +140,17 @@ func (daemon *Daemon) ContainerLogs(ctx context.Context, containerName string, c
 	return messageChan, nil
 }
 
-func (daemon *Daemon) getLogger(container *container.Container) (logger.Logger, error) {
-	if container.LogDriver != nil && container.IsRunning() {
-		return container.LogDriver, nil
+func (daemon *Daemon) getLogger(container *container.Container) (l logger.Logger, created bool, err error) {
+	container.Lock()
+	if container.State.Running {
+		l = container.LogDriver
 	}
-	return container.StartLogger()
+	container.Unlock()
+	if l == nil {
+		created = true
+		l, err = container.StartLogger()
+	}
+	return
 }
 
 // mergeLogConfig merges the daemon log config to the container's log config if the container's log driver is not specified.