Browse Source

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 <mickael.laventure@gmail.com>
Kenfe-Mickael Laventure 8 years ago
parent
commit
fb2bb3653e
2 changed files with 18 additions and 10 deletions
  1. 5 1
      daemon/logger/jsonfilelog/read.go
  2. 13 9
      daemon/logs.go

+ 5 - 1
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:
+		}
 	}
 }
 

+ 13 - 9
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