Skip to content

lz4: stop reading the source before a concurrent Reader returns - #292

Merged
pierrec merged 1 commit into
pierrec:v4from
honeycombio:lizf.lz4-reader-src-race
Oct 27, 2026
Merged

pierrec merged 1 commit into
pierrec:v4from
honeycombio:lizf.lz4-reader-src-race

Conversation

@lizthegrey

Copy link
Copy Markdown
Collaborator

Fixes #291. With ConcurrencyOption > 1, Reader could return while its read-ahead goroutine was still inside src.Read, so a caller that then reused, read or closed its source raced with it.

(1) Stop reading the source before a concurrent Reader returns (88fa3b4):

  • Read and WriteTo treated any empty buffer from the block channel as the end of the stream and returned ErrorR(). A block that decodes to nothing is also empty, so after a later block failed they returned its error while the read loop was still reading. They now check ErrorR() only when the channel is closed, which happens after the read loop has exited; an empty block just contributes nothing.
  • Reset abandoned the read-ahead without waiting, and so did WriteTo when the write to w failed. A new Blocks.StopR abandons it and waits on a stopped channel that the read loop closes once it is done with the source. Reset documents that it waits for a read of the previous source in progress, which with a stalled source (an idle net.Conn, say) is as long as that read takes.

TestReaderConcurrentStopsReadingSource in reader_test.go reads from a source that blocks reads past a gate, and checks that Read, WriteTo, WriteTo to a failing writer and Reset don't return while the read-ahead is blocked there. It doesn't need -race. All four cases fail on v4 and pass here, including 200 runs with -race. Removing only the reader.go change fails the first three; removing only the wait fails Reset and the failing writer.

go test ./... passes with and without -tags noasm, each with and without -race; go vet and go fix -diff are clean. FuzzReader with -race, which failed within seconds on v4, ran for 3 minutes clean, and FuzzReader and FuzzFrameRoundTrip ran for 3 minutes each with and without -tags noasm. The public-API repro from the issue passes with -race. No performance change is claimed: the hot path gains only the ok from the channel receive.

🤖 Generated with Claude Code

With concurrency, Read and WriteTo took any empty block as the end of
the stream and returned the error of a later block while the read-ahead
was still reading the source; they now wait for the block channel to
close. Reset, and WriteTo after a failed write, abandoned the read-ahead
without waiting; they now wait until it no longer uses the source, so
the caller may reuse it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lizthegrey
lizthegrey requested a review from pierrec October 26, 2026 06:04
@lizthegrey
lizthegrey marked this pull request as ready for review October 26, 2026 06:06
@pierrec
pierrec merged commit c3ed367 into pierrec:v4 Oct 27, 2026
14 checks passed
@lizthegrey
lizthegrey deleted the lizf.lz4-reader-src-race branch November 5, 2026 02:46
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.

Concurrent Reader keeps reading its source after Read/WriteTo return an error, and after Reset

2 participants