semaphore: add leased Redis semaphore#79
Merged
Conversation
On a fatal read error (e.g. the underlying stream key being destroyed) the reader read loop called Close synchronously. Close waits on the read goroutine via wait.Wait, so calling it from that same goroutine deadlocked and leaked the reader and its Redis connection; any external Close blocked forever too. Trigger the shutdown asynchronously so the read goroutine can return and release the wait group. Close is already idempotent via sync.Once, so this is safe alongside a concurrent external Close. Adds a regression test that drives the real read loop through a simulated fatal error (via a small xreadFn seam) and asserts the reader closes instead of hanging. The test hangs on pre-fix code.
Move the test-only read hook from a Reader struct field to a package variable so it no longer pollutes Reader. Behavior is unchanged; it defaults to (*Reader).xread and is only overridden in tests.
Refresh Go dependencies for the release candidate and document the new leased semaphore package so users can discover and adopt it.
Include the new semaphore package in the race-test script so release checks cover its Redis-backed lease behavior.
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
1 participant
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.
Summary
semaphorepackage for fleet-wide leased concurrency limits.go get -u -v ./...and includesemaphore/...in the CI race-test script.Test plan
REDIS_PORT=6380 ./scripts/cibuildRelease note intent
This is intended to ship as
v1.8.0: it adds a new public package without requiring changes for existing users.