From b642078c87ef0c71e2495e622c87f431428921b6 Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Wed, 20 Dec 2023 16:01:08 +0100
Subject: [PATCH] prevent "docker stats" from hanging if the initial API call
 fails

When running `docker stats` without a list of containers, `runStats` collects
an initial list of containers. If that API call fails, the error is sent to the
`closeChan`, however, `closeChan` is non-buffered, and nothing is reading the
channel until we received the initial list and start collecting stats.

This patch rewrites the code that gets the initial list of containers to
return the error if the API call fails. The `getContainerList` closure is
also removed and inlined to make the logic somewhat easier to read.

Before this patch, the command would hang without producing output;

    docker stats
    # hangs; no output

With this patch, the error is printed, and the CLI exits:

    docker stats
    Error response from daemon: some error occurred

    echo $?
    1

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 cli/command/container/stats.go | 45 +++++++++++++++-------------------
 1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/cli/command/container/stats.go b/cli/command/container/stats.go
index 6141ba1380..1d240cf44e 100644
--- a/cli/command/container/stats.go
+++ b/cli/command/container/stats.go
@@ -135,24 +135,6 @@ func runStats(ctx context.Context, dockerCLI command.Cli, options *statsOptions)
 			}
 		}
 
-		// getContainerList simulates creation event for all previously existing
-		// containers (only used when calling `docker stats` without arguments).
-		getContainerList := func() {
-			cs, err := apiClient.ContainerList(ctx, container.ListOptions{
-				All: options.all,
-			})
-			if err != nil {
-				closeChan <- err
-			}
-			for _, ctr := range cs {
-				s := NewStats(ctr.ID[:12])
-				if cStats.add(s) {
-					waitFirst.Add(1)
-					go collect(ctx, s, apiClient, !options.noStream, waitFirst)
-				}
-			}
-		}
-
 		eventChan := make(chan events.Message)
 		go eh.Watch(eventChan)
 		stopped := make(chan struct{})
@@ -160,17 +142,30 @@ func runStats(ctx context.Context, dockerCLI command.Cli, options *statsOptions)
 		defer close(stopped)
 		<-started
 
-		// Start a short-lived goroutine to retrieve the initial list of
-		// containers.
-		getContainerList()
+		// Fetch the initial list of containers and collect stats for them.
+		// After the initial list was collected, we start listening for events
+		// to refresh the list of containers.
+		cs, err := apiClient.ContainerList(ctx, container.ListOptions{
+			All: options.all,
+		})
+		if err != nil {
+			return err
+		}
+		for _, ctr := range cs {
+			s := NewStats(ctr.ID[:12])
+			if cStats.add(s) {
+				waitFirst.Add(1)
+				go collect(ctx, s, apiClient, !options.noStream, waitFirst)
+			}
+		}
 
 		// make sure each container get at least one valid stat data
 		waitFirst.Wait()
 	} else {
-		// Artificially send creation events for the containers we were asked to
-		// monitor (same code path than we use when monitoring all containers).
-		for _, name := range options.containers {
-			s := NewStats(name)
+		// Create the list of containers, and start collecting stats for all
+		// containers passed.
+		for _, ctr := range options.containers {
+			s := NewStats(ctr)
 			if cStats.add(s) {
 				waitFirst.Add(1)
 				go collect(ctx, s, apiClient, !options.noStream, waitFirst)