Skip to content

Fixed a possible leak bug in goroutine - #13108

Closed
suwakei wants to merge 1 commit into
docker:mainfrom
suwakei:fix/stopping.go
Closed

Fixed a possible leak bug in goroutine#13108
suwakei wants to merge 1 commit into
docker:mainfrom
suwakei:fix/stopping.go

Conversation

@suwakei

Copy link
Copy Markdown
Contributor

Problem:

The goroutine invoked by the ApplicationTermination method (go func() { ... }) has no termination mechanism.

The Close method calls s.ticker.Stop(), which only stops the ticker, but does not terminate the for loop in the goroutine.
The goroutine leaks because it is permanently waiting to receive from the channel in <-s.ticker.C.
This goroutine will remain in the program even after the command is terminated.

Fix:

Use context to manage the goroutine lifecycle.

By adding context.CancelFunc to the Stopping structure and notifying the goroutine of the cancellation with the Close method, the goroutine can be safely terminated.

Another small change is that within the ApplicationTermination method, the value 100 * time.Millisecond is written directly to time.NewTicker. This “magic number” could be confusing to the intent of the code and has been corrected.

package formatter
import (
+++ "context"
"fmt"
"strings"
"time"
"github.com/docker/compose/v2/pkg/progress"
)
+++ const tickerInterval = 100 * time.Millisecond
type Stopping struct {
api.LogConsumer
enabled bool
spinner *progress.Spinner
ticker *time.Ticker
startedAt time.Time
+++ cancel context.CancelFunc
}
func NewStopping(l api.LogConsumer) *Stopping {
s.spinner = progress.NewSpinner()
hideCursor()
s.startedAt = time.Now()
--- s.ticker = time.NewTicker(100 * time.Millisecond)
+++ s.ticker = time.NewTicker(tickerInterval)
ctx, cancel := context.WithCancel(context.Background())
s.cancel = cancel
go func() {
for {
--- <-s.ticker.C
--- s.print()
+++ select {
+++ case <-s.ticker.C:
+++ s.print()
+++ case <-ctx.Done():
+++ return
+++ }
}
}()
}
func (s *Stopping) Close() {
showCursor()
--- if s.ticker != nil {
--- s.ticker.Stop()
--- }
+++ s.ticker.Stop()
+++ s.cancel()
s.clear()
}

Benefits:

Applying this goroutine leak fix will greatly improve the stability of the program.

Signed-off-by: keitosuwahara <keitosuwahara0816@gmail.com>
@suwakei
suwakei requested a review from a team as a code ownerJuly 28, 2025 12:31
@suwakei
suwakei requested review from glours and ndeloofJuly 28, 2025 12:31
@ndeloof

Copy link
Copy Markdown
Contributor

this is obsolete after #13064 has been merged

@suwakei

Copy link
Copy Markdown
ContributorAuthor

Thank you for the feedback! Closing this PR since #13064 already addresses the issue.

@suwakeisuwakei closed this Jul 28, 2025
@suwakei
suwakei deleted the fix/stopping.go branch August 2, 2025 08:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@suwakei@ndeloof