Alanxtl commented on code in PR #3293:
URL: https://github.com/apache/dubbo-go/pull/3293#discussion_r3108356866
##########
server/server.go:
##########
@@ -356,7 +370,17 @@ func (s *Server) Serve() error {
probe.SetStartupComplete(true)
probe.SetReady(true)
- select {}
+ if done := ctx.Done(); done != nil {
+ select {
+ case <-graceful_shutdown.Done():
+ return graceful_shutdown.Shutdown(ctx)
+ case <-done:
+ return graceful_shutdown.Shutdown(ctx)
+ }
+ }
+
+ <-graceful_shutdown.Done()
+ return graceful_shutdown.Shutdown(context.Background())
}
Review Comment:
`ServeContext` only checks `ctx.Done()` after the server has already
exported services, registered the instance, and flipped readiness to true. So
an already-canceled context, or a timeout that fires during startup, still lets
the service become externally visible before shutdown begins. That makes the
new API a poor fit for caller-controlled lifecycle during startup.
##########
server/server.go:
##########
@@ -356,7 +370,17 @@ func (s *Server) Serve() error {
probe.SetStartupComplete(true)
probe.SetReady(true)
- select {}
+ if done := ctx.Done(); done != nil {
+ select {
+ case <-graceful_shutdown.Done():
+ return graceful_shutdown.Shutdown(ctx)
+ case <-done:
+ return graceful_shutdown.Shutdown(ctx)
+ }
+ }
Review Comment:
canceling `ServeContext` does not wait for graceful shutdown to finish.
Because the already-canceled `ctx` is forwarded into
`graceful_shutdown.Shutdown(ctx)`, the call can return `context.Canceled`
immediately while the real shutdown work is still running in the background.
The new test at server_test.go (line 186) actually codifies that behavior by
waiting for `ServeContext` to return first and only then waiting on
`graceful_shutdown.Done()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
