Alanxtl commented on code in PR #3293:
URL: https://github.com/apache/dubbo-go/pull/3293#discussion_r3090557510


##########
server/server.go:
##########
@@ -323,9 +328,29 @@ func (s *Server) Serve() error {
        }
        // prevent multiple calls to Serve
        s.serve = true
+       s.stopCh = make(chan struct{})
+       s.serveDone = make(chan struct{})
+       s.stopOnce = sync.Once{}
+       s.stopperID = 0
+       stopCh := s.stopCh
+       serveDone := s.serveDone
 
        // release lock in case causing deadlock
        s.mu.Unlock()
+       defer func() {
+               s.mu.Lock()
+               stopperID := s.stopperID
+               if s.serveDone == serveDone {
+                       s.serve = false
+                       s.stopCh = nil
+                       s.serveDone = nil
+                       s.stopperID = 0
+               }
+               s.mu.Unlock()
+
+               graceful_shutdown.UnregisterServerStopper(stopperID)
+               close(serveDone)
+       }()

Review Comment:
   为啥这里把s.serveDone赋值给了serveDone 然后defer里面比较了s.serveDone和serveDone 
   
   这个s.serveDone在运行时会被修改吗



##########
graceful_shutdown/shutdown.go:
##########
@@ -130,6 +183,24 @@ func RegisterProtocol(name string) {
        proMu.Unlock()
 }
 
+// Shutdown runs the process-level graceful shutdown orchestration once.
+// Concurrent or repeated callers wait on the same execution result, while 
their
+// own contexts only control how long they wait for that shared run to finish.
+func Shutdown(ctx context.Context) error {
+       if ctx == nil {
+               ctx = context.Background()
+       }
+
+       startShutdownOnce()
+
+       select {
+       case <-shutdownDone:
+               return shutdownResult
+       case <-ctx.Done():
+               return ctx.Err()
+       }
+}

Review Comment:
   `Shutdown()` 对 stopper 只做了一次快照,如果某个 server 在 shutdown 已开始但快照之后才完成启动并注册 
stopper,它就不会被停掉,而 `Serve()` 又会因为 `<-graceful_shutdown.Done()` 直接返回,留下活着的服务实例。



##########
server/server.go:
##########
@@ -352,11 +377,50 @@ func (s *Server) Serve() error {
                return err
        }
 
+       s.mu.Lock()
+       s.stopperID = graceful_shutdown.RegisterServerStopper(func(ctx 
context.Context) error {
+               return s.Stop(ctx)
+       })
+       s.mu.Unlock()
+
        // k8s probe ready
        probe.SetStartupComplete(true)
        probe.SetReady(true)
 
-       select {}
+       select {
+       case <-stopCh:
+       case <-graceful_shutdown.Done():
+       }
+
+       return nil
+}
+
+func (s *Server) Stop(ctx context.Context) error {
+       if ctx == nil {
+               ctx = context.Background()
+       }
+
+       s.mu.Lock()
+       stopCh := s.stopCh
+       done := s.serveDone
+       stopperID := s.stopperID
+       if stopCh == nil || done == nil {
+               s.mu.Unlock()
+               return nil
+       }
+       s.stopOnce.Do(func() {
+               close(stopCh)
+       })
+       s.mu.Unlock()
+
+       graceful_shutdown.UnregisterServerStopper(stopperID)
+
+       select {
+       case <-done:
+               return nil
+       case <-ctx.Done():
+               return ctx.Err()
+       }
 }

Review Comment:
   `Server.Stop(ctx)` 现在只是“让 `Serve()` 退出阻塞”,并不是真正的 server shutdown。代码没有调用 
graceful-shutdown 流程,所以手动 stop 后 exporter、监听端口、注册中心状态都还在。
   



-- 
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]

Reply via email to