-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix race issue #22
Fix race issue #22
Conversation
This reverts commit b70cdfa.
@alixander, it's ready for merge, and we can test it afterward. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
u overwrote the revert?
yea i did... |
@alixander can we test now ? or is there a problem... |
xhttp/serve.go
Outdated
@@ -41,11 +81,12 @@ func Serve(ctx context.Context, shutdownTimeout time.Duration, s *http.Server, l | |||
shutdownCtx, cancel := context.WithTimeout(xcontext.WithoutCancel(ctx), shutdownTimeout) | |||
defer cancel() | |||
|
|||
err := s.Shutdown(shutdownCtx) | |||
err := ss.Shutdown(shutdownCtx) | |||
<-serverClosed // Wait for server to exit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the previous changes were not reverted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alixander can you just do a reset on the master branch ?
tho i really don't understand why a revert is necessary here.
fix: Resolve race condition in HTTP server handling
safeServer
to wrap http.ServersafeServer
for improved concurrencyThis change addresses a race condition in the HTTP server handling,particularly affecting CLI tests. By introducing a
safeServer
struct with atomic operations and proper synchronization, and prevent potential data races during server startup and shutdown.Fix for terrastruct/d2#2087