Skip to content

progress: avoid panic when closing printer#3873

Open
lohitkolluri wants to merge 1 commit into
docker:masterfrom
lohitkolluri:lk/fix-progress-printer
Open

progress: avoid panic when closing printer#3873
lohitkolluri wants to merge 1 commit into
docker:masterfrom
lohitkolluri:lk/fix-progress-printer

Conversation

@lohitkolluri
Copy link
Copy Markdown

Summary

  • Prevent send on closed channel panics in progress Printer shutdown paths by making status writes best-effort during close.

Test plan

  • go test ./util/progress -short

Guard progress status writes during shutdown to prevent send-on-closed-channel panics observed in remote integration tests.

Signed-off-by: Lohit Kolluri <lohitkolluri@gmail.com>
Copy link
Copy Markdown
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

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

Thanks for the patch. Before changing the Printer write semantics, can you open an issue with a current reproduction and link it here?

Right now this PR doesn't show the lifecycle that can call Write after Wait has closed p.status, and there is no regression test. The only matching report I found is #1176, which is from 2022 and was closed as stale/possibly SIGTERM-related.

This patch also changes more than shutdown behavior: Write becomes best-effort and can drop progress updates whenever the buffer is full, not only while closing. That can lose logs/status/warnings during normal operation.

If this panic is still reproducible, please include the command, buildx/BuildKit versions, progress mode, a minimized Dockerfile or bake file if possible, and the full stack trace. Then we can add a targeted failing test and fix the actual lifecycle race rather than making all progress writes lossy.

@lohitkolluri
Copy link
Copy Markdown
Author

Opened an issue with a current reproduction + full stack trace here: #3875

It reproduces in CI under test-integration (v0.28.1, remote, ./tests, experimental) and points at a lifecycle race where Write() can run after Wait() has closed p.status.

Next step on my side: add a targeted regression test for that specific lifecycle and adjust the shutdown/lifecycle handling, rather than making all progress writes lossy in normal operation.

@lohitkolluri
Copy link
Copy Markdown
Author

Linking for tracking: #3875

@lohitkolluri lohitkolluri requested a review from crazy-max May 28, 2026 13:58
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.

3 participants