Skip to content

Conversation

@Amr-Shams
Copy link
Contributor

Motivation / Background

This PR addresses a concurrency-related bug in Subscriber.read() where slices returned by bufio.Reader.ReadSlice were shared between iterations, leading to race conditions when the consumer of the channel retained or mutated the chunk after it was reused by the reader.

This issue has been documented in #581. The race occurs because ReadSlice reuses an internal buffer, so passing the slice directly through the channel can lead to unsafe access if it is used concurrently elsewhere.

Fixes #581

Details

To resolve the race condition, the read loop now copies the chunk into a new slice before sending it over the channel. This ensures that each message chunk sent to downstream consumers is a safe, isolated copy that won't be invalidated by future reads.

Benchmarking showed a negligible decrease in throughput in exchange for guaranteed safety in concurrent environments.

Alternative approaches considered

Checklist

  • I wrote tests for the changes.
  • All tests are passing.
    • If you are testing a Pub/Sub, you can start Docker with make up.
    • You can start with make test_short for a quick check.
    • If you want to run all tests, use make test.
  • Code has no breaking changes.
  • (If applicable) documentation on watermill.io is updated.

@roblaszczak roblaszczak merged commit 88fdc39 into ThreeDotsLabs:master Aug 25, 2025
@roblaszczak
Copy link
Member

Cool, thanks @Amr-Shams for the contribution!

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.

2 participants