fix(http-server): remove data race between Start and Shutdown - #9
Open
rzatonskiy wants to merge 4 commits into
Open
rzatonskiy wants to merge 4 commits into
rzatonskiy wants to merge 4 commits into
Conversation
* pass the e.Start result through a buffered channel instead of the named result * treat http.ErrServerClosed as a clean stop and return nil * return an early start failure without waiting for the context * join the serving goroutine after the shutdown and report its error * cover both paths with tests runnable under -race
* add -race to the test script, so a reintroduced race fails the build * raise the go matrix to 1.23-1.27, the module requires go 1.23.0
* the di container is not safe for concurrent resolution, do it sequentially * a resolve failure now fails before a single server is started, so no server is left running behind an error
…utdown * report a shutdown failure as is: the listener may still be open, and echo.Shutdown gives up on the plain server whenever the TLS one fails, so the join would hang * pin the start failure down to a listen error in the test, not to any error at all * assert the address is free once the command returned, so a shutdown that never happened cannot pass * keep the echo banner out of the test output and stop sharing the default transport
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.
The http-server command declared a named result and wrote to it from two goroutines:
e.Start in the serving goroutine and e.Shutdown after the context cancellation. Every
graceful stop raced, and the command returned nil or http.ErrServerClosed depending on
which write landed last.
until the process is interrupted
a started server behind and the container is not resolved concurrently
The CI matrix was pinned to go 1.18/1.19 while go.mod requires go 1.23.0, so it is
raised to 1.23-1.27.