Skip to content

fix(server): replace select{} in Serve() with graceful lifecycle management#3321

Open
cvictory wants to merge 1 commit into
mainfrom
task/serve-graceful-shutdown-20260511-001500
Open

fix(server): replace select{} in Serve() with graceful lifecycle management#3321
cvictory wants to merge 1 commit into
mainfrom
task/serve-graceful-shutdown-20260511-001500

Conversation

@cvictory
Copy link
Copy Markdown
Contributor

Bug

  • Symptom: Server.Serve() uses select {} to block forever, making it impossible to shut down gracefully from code or respond to lifecycle signals.
  • Root cause: hard spin (select {}) has no termination path.

Fix

  • Add stopCh chan struct{} to Server for lifecycle signaling.
  • NewServer initializes stopCh.
  • Serve() blocks on <-stopCh instead of select{}.
  • Add Stop() method: safe, idempotent, and allows Serve() to be called again after shutdown.
  • Serve() re-creates stopCh if previously closed, supporting restart.
  • Add serve_stop_test.go with 4 lifecycle tests (all pass).

Verification

  • Regression tests: all 148 existing server tests pass.
  • New tests: TestStopBeforeServe, TestStopIdempotent, TestServeStopGracefulShutdown, TestServeAfterStop all pass.

Fixes #3042

…gement

Replace the hard-blocking select{} in Server.Serve() with a stopCh
channel, enabling external callers to trigger graceful shutdown via
the new Server.Stop() method.

Changes:
- Add stopCh field to Server struct for lifecycle signaling
- NewServer now initializes stopCh
- Serve() blocks on <-stopCh instead of select{}
- Add Stop() method: safe, idempotent, allows Serve() restart
- Serve() re-creates stopCh if previously closed, supporting restart
- Add serve_stop_test.go with 4 lifecycle tests

Fixes #3042
@sonarqubecloud
Copy link
Copy Markdown

@Alanxtl Alanxtl added the 3.3.2 version 3.3.2 label May 11, 2026
Copy link
Copy Markdown
Contributor

@Alanxtl Alanxtl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

server/server.go:379-386 only flips serve=false and closes stopCh. But Serve() has already exported services at server/server.go:355-358 and registered the service instance at server/server.go:361; none of those resources are unexported/deregistered/destroyed. Existing teardown lives in paths like ServiceOptions.Unexport() (server/action.go:310), but Stop() never calls it. Result: Serve() returns, yet listeners and registry exposure can remain alive, so this is not a graceful shutdown and “Serve can be called again after shutdown” is misleading. The new tests miss this because they use an empty NewServer() with no exported service. Please add a real exported Triple/Dubbo service test: call Serve(), verify it accepts traffic, call Stop(), then verify the port/request is closed or fails and registry/protocol cleanup happened.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.3.2 version 3.3.2 ☢️ Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[提案] server/server.go中Serve优化

2 participants