Skip to content

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

@github-actions

Description

@github-actions

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 #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

Metadata

Metadata

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions