Skip to content

Fix panic safety and deduplicate cleanup in StartDockerImageDownload goroutine#31163

Merged
pelikhan merged 2 commits into
mainfrom
copilot/fix-docker-pull-goroutine-recovery
May 9, 2026
Merged

Fix panic safety and deduplicate cleanup in StartDockerImageDownload goroutine#31163
pelikhan merged 2 commits into
mainfrom
copilot/fix-docker-pull-goroutine-recovery

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 9, 2026

Bug Fix

What was the bug?

The background goroutine in StartDockerImageDownload had no panic recovery — any unhandled panic would crash the entire process. Compounding this, the pullState.downloading[image] cleanup was open-coded on every return path (four copies), meaning a panic would also leave the flag set permanently, silently no-op-ing all future download attempts for that image until process restart.

Every sibling goroutine in the package (update_check.go, compile_update_check.go) already uses defer recover() with logging; this goroutine was the odd one out.

How did you fix it?

Replaced the four duplicate lock → delete → unlock blocks with a single defer that performs cleanup unconditionally and recovers from panics:

go func() {
    defer func() {
        pullState.mu.Lock()
        delete(pullState.downloading, image)
        pullState.mu.Unlock()
        if r := recover(); r != nil {
            dockerImagesLog.Printf("Panic in docker image download for %s (recovered): %v", image, r)
        }
    }()
    // retry loop — all four early returns now just `return`
}()

No semantic change on success or cancellation paths; panic path now recovers instead of crashing.

…wnload goroutine

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix panic recovery and duplicate cleanup in Docker pull goroutine Fix panic safety and deduplicate cleanup in StartDockerImageDownload goroutine May 9, 2026
Copilot AI requested a review from pelikhan May 9, 2026 05:22
@pelikhan pelikhan marked this pull request as ready for review May 9, 2026 05:30
Copilot AI review requested due to automatic review settings May 9, 2026 05:30
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the robustness of StartDockerImageDownload by ensuring the background download goroutine always cleans up its pullState.downloading flag and won’t crash the process if it panics.

Changes:

  • Added a deferred cleanup in the download goroutine to always remove the image from pullState.downloading.
  • Added recover() handling to log and recover from panics inside the goroutine.
  • Removed duplicated cleanup blocks across multiple return paths.
Show a summary per file
File Description
pkg/cli/docker_images.go Adds deferred cleanup + panic recovery to the docker image pull goroutine and removes duplicated map cleanup code.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@pelikhan pelikhan merged commit 9b622c2 into main May 9, 2026
4 checks passed
@pelikhan pelikhan deleted the copilot/fix-docker-pull-goroutine-recovery branch May 9, 2026 05:38
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.

Background Docker pull goroutine lacks panic recovery and duplicates cleanup four times

3 participants