logutil: fix data race when Pause is called concurrently - #4007
Draft
alexey-igrychev wants to merge 1 commit into
Draft
logutil: fix data race when Pause is called concurrently#4007alexey-igrychev wants to merge 1 commit into
alexey-igrychev wants to merge 1 commit into
Conversation
Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
alexey-igrychev
force-pushed
the
fix/concurrent-log-pause
branch
from
August 10, 2026 13:27
8ea2e36 to
7faf88b
Compare
This was referenced Aug 10, 2026
alexey-igrychev
added a commit
to werf/werf
that referenced
this pull request
Aug 10, 2026
…#7803) ## Summary Branch `3` currently serializes concurrent Dockerfile builds: the `werf/3p-buildx` replacement in `go.mod` fixes a data race in `docker/buildx`'s `logutil.Pause` with a global mutex that is held for a build's entire progress-display lifetime, so two Dockerfile builds in one process cannot run at the same time. This points the replacement at a fork commit that fixes the same race without a global lock, restoring concurrency. ## What - Concurrent Dockerfile builds in a single werf process run in parallel again; the previous fork serialized them. - The `-race`-detected data race on `logrus.StandardLogger()` in `logutil.Pause` stays fixed: pauses are multiplexed through one refcounted per-logger writer, and `Pause` never blocks on another active pause. - Only the `go.mod`/`go.sum` `docker/buildx => werf/3p-buildx` pin changes; no CLI, configuration, or persisted-data changes. ## Why #7800 removed this replacement precisely because its global mutex serialized concurrent builds, and said to keep it out until an upstream-safe fix existed. #7802 was branched before that revert and reintroduced the old pin on merge, so `3` regressed to the serializing fork. The new fork commit (backport of docker/buildx#4007) replaces the global mutex with a per-logger refcounting writer: the first `Pause` wraps the logger output once, each `Pause` bumps a counter, output is buffered while any pause is active and flushed in order when the last one resumes. Remove this replacement once the fix lands in an upstream buildx release. Fixes: f80e827 ("fix(logboek): prevent concurrent stream races (#7802)") Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
logutil.Pausemutates the shared logger's output on every call:Printer.updateDisplaycallsPause(logrus.StandardLogger())and holds it(via
defer resumeLogs()) for the printer's entire active period. When morethan one
Printerruns at the same time in a single process — e.g. a toolthat drives several concurrent
buildoperations — each one readsl.Outand calls
l.SetOutputon the same process-widelogrus.StandardLogger()without synchronization:
Fix
Install a single refcounting writer per logger instead of swapping the
logger's output on every
Pause:Pausefor a logger wrapsl.Outonce (formatter terminal-initstill happens while
l.Outis the real terminal) and stores the wrapper ina small guarded map.
Pauseincrements a counter; each resume decrements it. Output isbuffered while the count is > 0 and flushed, in order, when it returns to 0.
Pausenever blocks on another active pause, so concurrent printers keeprunning in parallel — only the interleaving of logrus output with the
progress display is serialized, not the work itself.
Test
util/logutil/pause_test.go(new) covers concurrent pause/resume, concurrentfirst-time initialization (the raced path), overlapping-output buffering, and
resume idempotency.
go test -race ./util/logutilfails on master and passeswith this change.