Skip to content

Pass the tar error over a channel in 'mtree' - #425

Merged
folbricht merged 1 commit into
masterfrom
fix-mtree-tarerr-race
Sep 3, 2026
Merged

folbricht merged 1 commit into
masterfrom
fix-mtree-tarerr-race

Conversation

@folbricht

Copy link
Copy Markdown
Owner

Walking a directory runs Tar in a goroutine writing into a pipe while UnTar reads it. UnTar can stop before the stream ends — a cancelled context being one way — and the main goroutine then read tarErr while the Tar goroutine was still assigning it. The equivalent in tar.go is safe only because ChunkStream cannot return nil before the pipe writer closes.

The error now goes over a channel, and the read end of the pipe is closed first so Tar is not left blocked writing into a pipe nobody reads, which is what makes waiting for it safe.

mtree had no tests, so this adds them for all three input kinds. The cancelled-context case reproduces the race: go test -race reports DATA RACE on it before the change and passes twenty consecutive runs after. CI already runs the race detector over the package.

The command writes to the package's stdout writer rather than os.Stdout directly, which is what lets the output be checked at all.

Walking a directory runs Tar in a goroutine writing into a pipe while
UnTar reads it. UnTar can stop before the stream ends, a cancelled
context being one way, and the main goroutine then read tarErr while the
Tar goroutine was still assigning it.

Send the error over a channel instead, and close the read end of the pipe
first so Tar isn't left blocked writing into it and can be waited for.

The command writes to the package's stdout writer, which is what lets it
be tested at all; these are its first tests.
@folbricht
folbricht merged commit ddc1695 into master Sep 3, 2026
9 checks passed
@folbricht
folbricht deleted the fix-mtree-tarerr-race branch September 3, 2026 10:57
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.

1 participant

⌂ 换网址