From a6ade7e78d30c63a4a870074fe3f5d3ffbb704f2 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sun, 26 May 2024 11:26:16 +1000 Subject: [PATCH 1/5] Close stream when done with it We should have been doing this from the start. Potential cause of a memory leak? --- pkg/gui/container_logs.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/gui/container_logs.go b/pkg/gui/container_logs.go index bb2feb85..95144478 100644 --- a/pkg/gui/container_logs.go +++ b/pkg/gui/container_logs.go @@ -114,8 +114,10 @@ func (gui *Gui) writeContainerLogs(container *commands.Container, ctx context.Co Follow: true, }) if err != nil { + gui.Log.Error(err) return err } + defer readCloser.Close() if container.DetailsLoaded() && container.Details.Config.Tty { _, err = io.Copy(writer, readCloser) From bca15cf6b1b6d1b31a8c845ee8e1e26ad00903eb Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sun, 26 May 2024 12:19:17 +1000 Subject: [PATCH 2/5] Get container details immediately after getting containers Previously we separately obtained container details every second, meaning there was a second of not knowing whether a given container was using a TTY or not, and that meant we were using the wrong streaming method for logs, which led to a black screen. --- pkg/commands/container.go | 4 +++- pkg/commands/docker.go | 38 ++++++++++++++++++++++++++------------ pkg/gui/gui.go | 2 +- 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/pkg/commands/container.go b/pkg/commands/container.go index b45feef7..132e4db3 100644 --- a/pkg/commands/container.go +++ b/pkg/commands/container.go @@ -140,7 +140,9 @@ func (c *Container) RenderTop(ctx context.Context) (string, error) { return utils.RenderTable(append([][]string{result.Titles}, result.Processes...)) } -// DetailsLoaded tells us whether we have yet loaded the details for a container. Because this is an asynchronous operation, sometimes we have the container before we have its details. +// DetailsLoaded tells us whether we have yet loaded the details for a container. +// Sometimes it takes some time for a container to have its details loaded +// after it starts. func (c *Container) DetailsLoaded() bool { return c.Details.ContainerJSONBase != nil } diff --git a/pkg/commands/docker.go b/pkg/commands/docker.go index e96c9653..7e4b3c5b 100644 --- a/pkg/commands/docker.go +++ b/pkg/commands/docker.go @@ -10,6 +10,7 @@ import ( "os" "os/exec" "strings" + "sync" "time" cliconfig "github.com/docker/cli/cli/config" @@ -209,7 +210,7 @@ func (c *DockerCommand) GetContainers(existingContainers []*Container) ([]*Conta for i, container := range containers { var newContainer *Container - // check if we already data stored against the container + // check if we already have data stored against the container for _, existingContainer := range existingContainers { if existingContainer.ID == container.ID { newContainer = existingContainer @@ -244,6 +245,8 @@ func (c *DockerCommand) GetContainers(existingContainers []*Container) ([]*Conta ownContainers[i] = newContainer } + c.SetContainerDetails(ownContainers) + return ownContainers, nil } @@ -278,24 +281,35 @@ func (c *DockerCommand) GetServices() ([]*Service, error) { return services, nil } -// UpdateContainerDetails attaches the details returned from docker inspect to each of the containers -// this contains a bit more info than what you get from the go-docker client -func (c *DockerCommand) UpdateContainerDetails(containers []*Container) error { +func (c *DockerCommand) RefreshContainerDetails(containers []*Container) error { c.ContainerMutex.Lock() defer c.ContainerMutex.Unlock() - for _, container := range containers { - details, err := c.Client.ContainerInspect(context.Background(), container.ID) - if err != nil { - c.Log.Error(err) - } else { - container.Details = details - } - } + c.SetContainerDetails(containers) return nil } +// Attaches the details returned from docker inspect to each of the containers +// this contains a bit more info than what you get from the go-docker client +func (c *DockerCommand) SetContainerDetails(containers []*Container) { + wg := sync.WaitGroup{} + for _, container := range containers { + container := container + wg.Add(1) + go func() { + details, err := c.Client.ContainerInspect(context.Background(), container.ID) + if err != nil { + c.Log.Error(err) + } else { + container.Details = details + } + wg.Done() + }() + } + wg.Wait() +} + // ViewAllLogs attaches to a subprocess viewing all the logs from docker-compose func (c *DockerCommand) ViewAllLogs() (*exec.Cmd, error) { cmd := c.OSCommand.ExecutableFromString( diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 4797fb7f..281dd459 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -294,7 +294,7 @@ func (gui *Gui) setPanels() { } func (gui *Gui) updateContainerDetails() error { - return gui.DockerCommand.UpdateContainerDetails(gui.Panels.Containers.List.GetAllItems()) + return gui.DockerCommand.RefreshContainerDetails(gui.Panels.Containers.List.GetAllItems()) } func (gui *Gui) refresh() { From 623fdb4f335d02b37e10df63146edff53582238f Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sun, 26 May 2024 12:25:32 +1000 Subject: [PATCH 3/5] Use taskID mutex properly --- pkg/tasks/tasks.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/tasks/tasks.go b/pkg/tasks/tasks.go index e6b9d4ac..06fac38f 100644 --- a/pkg/tasks/tasks.go +++ b/pkg/tasks/tasks.go @@ -65,9 +65,12 @@ func (t *TaskManager) NewTask(f func(ctx context.Context)) error { t.waitingMutex.Lock() defer t.waitingMutex.Unlock() + t.taskIDMutex.Lock() if taskID < t.newTaskId { + t.taskIDMutex.Unlock() return } + t.taskIDMutex.Unlock() ctx, cancel := context.WithCancel(context.Background()) notifyStopped := make(chan struct{}) From 8a0fcf99f4eed7ce25d7cfbd443ade9b243f1ab3 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sun, 26 May 2024 12:27:01 +1000 Subject: [PATCH 4/5] Gracefully handle situation when container details have not yet loaded This should be very rare but it does happen sometimes --- pkg/gui/container_logs.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/pkg/gui/container_logs.go b/pkg/gui/container_logs.go index 95144478..2a1d4ecc 100644 --- a/pkg/gui/container_logs.go +++ b/pkg/gui/container_logs.go @@ -119,7 +119,24 @@ func (gui *Gui) writeContainerLogs(container *commands.Container, ctx context.Co } defer readCloser.Close() - if container.DetailsLoaded() && container.Details.Config.Tty { + if !container.DetailsLoaded() { + // loop until the details load or context is cancelled, using timer + ticker := time.NewTicker(time.Millisecond * 100) + defer ticker.Stop() + outer: + for { + select { + case <-ctx.Done(): + return nil + case <-ticker.C: + if container.DetailsLoaded() { + break outer + } + } + } + } + + if container.Details.Config.Tty { _, err = io.Copy(writer, readCloser) if err != nil { return err From 630ea7d32437e41d20c7f3abcc698abaed3d27ec Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sun, 26 May 2024 13:02:54 +1000 Subject: [PATCH 5/5] Fix issue where logs were buggy after returning from subprocess Turns out that upon switching to a subprocess we were cancelling ALL periodic tasks, which includes re-rendering the main view ever 30ms. This completely broke our log rendering and meant that it only updated when an event occurred --- pkg/gui/gui.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 281dd459..bf00fdd7 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -176,11 +176,9 @@ func (gui *Gui) goEvery(interval time.Duration, function func() error) { ticker := time.NewTicker(interval) defer ticker.Stop() for range ticker.C { - if gui.PauseBackgroundThreads { - return + if !gui.PauseBackgroundThreads { + _ = function() } - - _ = function() } }() }