Skip to content

Add context cancellation and WaitGroup tracking to background goroutines#12268

Merged
pelikhan merged 3 commits intomainfrom
copilot/add-context-cancellation-goroutines
Jan 28, 2026
Merged

Add context cancellation and WaitGroup tracking to background goroutines#12268
pelikhan merged 3 commits intomainfrom
copilot/add-context-cancellation-goroutines

Conversation

Copy link
Contributor

Copilot AI commented Jan 28, 2026

Background goroutines for docker image downloads, UI spinners, and update checks lacked cancellation mechanisms and completion tracking, causing potential goroutine leaks and terminal corruption on early exit.

Changes

  • Docker image downloads - StartDockerImageDownload() now accepts context.Context, uses exec.CommandContext(), and checks cancellation in retry loops with context-aware sleeps

  • Spinner lifecycle - SpinnerWrapper tracks goroutine with sync.WaitGroup, Stop() and StopWithMessage() now wait for completion before returning

  • Update checks - CheckForUpdatesAsync() accepts context, exits early on cancellation, uses context-aware select for timeout

Example

// Before: no cancellation, potential leak
go func() {
    cmd := exec.Command("docker", "pull", image)
    output, err := cmd.CombinedOutput()
    // ...
}()

// After: context-aware, cancellable
go func() {
    if ctx.Err() != nil {
        return
    }
    cmd := exec.CommandContext(ctx, "docker", "pull", image)
    output, err := cmd.CombinedOutput()
    // Check ctx.Err() in retry loop
}()

All callers updated to pass context.Context. Tests added for cancellation behavior. Passes -race detector.

Original prompt

This section details on the original issue you should resolve

<issue_title>[Code Quality] Add context cancellation and WaitGroup tracking to background goroutines</issue_title>
<issue_description>### Description

Add context-aware lifecycle management to background goroutines to prevent goroutine leaks and enable graceful shutdown. Currently, background goroutines for docker image downloads and UI spinners lack cancellation mechanisms and completion tracking.

Current Issues

Goroutine Leak Risk in Docker Image Downloads:

  • Location: pkg/cli/docker_images.go:95
  • Issue: Background goroutine spawned without context cancellation
  • Impact: Orphaned goroutines if main program exits during download

Missing WaitGroup in Spinner Lifecycle:

  • Location: pkg/console/spinner.go:124
  • Issue: Bubble Tea spinner goroutine spawned without tracking
  • Impact: Program may exit before spinner completes cleanup, corrupting terminal state

Suggested Changes

1. Docker Image Downloads (pkg/cli/docker_images.go)

  • Modify StartDockerImageDownload() to accept context.Context parameter
  • Use exec.CommandContext() instead of exec.Command()
  • Add context cancellation checks in retry loop
  • Handle context cancellation gracefully with cleanup

2. Spinner Lifecycle (pkg/console/spinner.go)

  • Add sync.WaitGroup field to SpinnerWrapper struct
  • Track spinner goroutine with wg.Add(1) / defer wg.Done()
  • Wait for goroutine completion in Stop() method before returning
  • Ensure proper cleanup sequence

3. Update Check (pkg/cli/update_check.go:232)

  • Apply similar context-aware patterns to async update check goroutine

Files Affected

  • pkg/cli/docker_images.go (line 95)
  • pkg/console/spinner.go (line 124)
  • pkg/cli/update_check.go (line 232)

Success Criteria

  • ✅ All background goroutines accept context.Context parameter
  • ✅ Docker pulls use exec.CommandContext() for cancellation support
  • ✅ Spinner goroutine tracked with WaitGroup
  • Stop() methods wait for goroutine completion before returning
  • ✅ Ctrl+C handling works correctly in integration tests
  • ✅ All tests pass with -race flag (no data races)
  • ✅ No goroutine leaks detected in tests

Benefits

  • Prevents goroutine leaks on program termination
  • Enables graceful shutdown with proper cleanup
  • Improves resource management in long-running processes
  • Better terminal state handling for spinners

Testing Strategy

  • Add unit test for context cancellation during docker pull
  • Add test for spinner Stop() racing with Start() goroutine
  • Run with -race flag to detect data races
  • Test Ctrl+C handling in integration tests
  • Add goroutine leak detection to test suite

Source

Extracted from Sergo Report discussion githubnext/gh-aw#12225 - "Finding 1: Goroutine Leak Risk" and "Finding 2: Missing WaitGroup" sections.

The report identifies these as HIGH severity issues: "If main program exits during download, goroutine becomes orphaned. In long-running processes, accumulates leaked goroutines."

Priority

High - Prevents resource leaks, improves stability, medium effort (2-3 days)

AI generated by Discussion Task Miner - Code Quality Improvement Agent

  • expires on Feb 11, 2026, 1:22 PM UTC

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits January 28, 2026 14:18
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Add context cancellation and WaitGroup tracking to background goroutines Add context cancellation and WaitGroup tracking to background goroutines Jan 28, 2026
Copilot AI requested a review from pelikhan January 28, 2026 14:26
@pelikhan pelikhan marked this pull request as ready for review January 28, 2026 14:28
@pelikhan pelikhan merged commit 4c40a36 into main Jan 28, 2026
124 checks passed
@pelikhan pelikhan deleted the copilot/add-context-cancellation-goroutines branch January 28, 2026 14:48
Sign up for free to 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.

[Code Quality] Add context cancellation and WaitGroup tracking to background goroutines

2 participants