Repository: zeromicro/go-zero (go)

Issue:
Graceful Exit Failure When Using Gateway and RPC Services Simultaneously within ServiceGroup
**Describe the bug**
I am encountering an issue where services fail to exit gracefully when both gateway and RPC services are used together within a ServiceGroup. The problem does not occur when the gateway service is removed from the group.
**To Reproduce**
Steps to reproduce the behavior, if applicable:
```go
	serviceGroup := service.NewServiceGroup()
	defer serviceGroup.Stop()
	rpcServer := zrpc.MustNewServer(c.RpcServer, func(grpcServer *grpc.Server) {
		pb.RegisterFlowServer(grpcServer, flowserver.NewFlowServer(svcCtx))
		reflection.Register(grpcServer)
	})
	serviceGroup.Add(rpcServer) 

	gw := gateway.MustNewServer(c.Gateway)
	serviceGroup.Add(gw)

	serviceGroup.Start()
	slog.Info("exiting")
```
Attempt to stop the services gracefully by CTRL+C

2. The error is

   
   the system does not print out the "Exiting" message that indicates a successful graceful shutdown process.
   

**Expected behavior**
print out the "Exiting" message 



**Environments (please complete the following information):**
 - OS: [e.g. Linux]
 - go-zero version [e.g. 1.6.6]


**More description**
Through debugging, I have identified two key issues that prevent graceful shutdown:

Gateway WaitGroup Blockage:
The gateway service appears to block on a waitgroup because not all of its shutdown listeners (registered via proc) have completed their shutdown procedures. This results in the gateway waiting indefinitely for these listeners to finish, preventing a graceful exit.
RPC Service Connection Hang:
The RPC service is unable to exit gracefully due to a persistent connection that refuses to close. This connection is associated with the gateway service, suggesting a possible deadlock scenario where each service is waiting on the other to complete its shutdown procedure. This cyclic dependency prevents both services from terminating properly.
These insights indicate that there might be a coordination issue between the gateway and RPC services during the shutdown process, possibly due to improper handling of shutdown signals or synchronization primitives like waitgroups.


Pull request:
fix: service group not working well when callback takes long time
fix #4261 

Patch:
diff --git a/core/proc/shutdown.go b/core/proc/shutdown.go
index 763742a2..71ce429a 100644
--- a/core/proc/shutdown.go
+++ b/core/proc/shutdown.go
@@ -82,6 +82,9 @@ func (lm *listenerManager) addListener(fn func()) (waitForCalled func()) {
 	})
 	lm.lock.Unlock()
 
+	// we can return lm.waitGroup.Wait directly,
+	// but we want to make the returned func more readable.
+	// creating an extra closure would be negligible in practice.
 	return func() {
 		lm.waitGroup.Wait()
 	}
diff --git a/core/service/servicegroup.go b/core/service/servicegroup.go
index 031abb5d..9281fb29 100644
--- a/core/service/servicegroup.go
+++ b/core/service/servicegroup.go
@@ -76,9 +76,14 @@ func (sg *ServiceGroup) doStart() {
 }
 
 func (sg *ServiceGroup) doStop() {
+	group := threading.NewRoutineGroup()
 	for _, service := range sg.services {
-		service.Stop()
+		// new variable to avoid closure problems, can be removed after go 1.22
+		// see https://golang.org/doc/faq#closures_and_goroutines
+		service := service
+		group.Run(service.Stop)
 	}
+	group.Wait()
 }
 
 // WithStart wraps a start func as a Service.
diff --git a/gateway/server.go b/gateway/server.go
index 71d1e554..51c11a35 100644
--- a/gateway/server.go
+++ b/gateway/server.go
@@ -11,6 +11,7 @@ import (
 	"github.com/jhump/protoreflect/grpcreflect"
 	"github.com/zeromicro/go-zero/core/logx"
 	"github.com/zeromicro/go-zero/core/mr"
+	"github.com/zeromicro/go-zero/core/threading"
 	"github.com/zeromicro/go-zero/gateway/internal"
 	"github.com/zeromicro/go-zero/rest"
 	"github.com/zeromicro/go-zero/rest/httpx"
@@ -23,6 +24,7 @@ type (
 	Server struct {
 		*rest.Server
 		upstreams     []Upstream
+		conns         []zrpc.Client
 		processHeader func(http.Header) []string
 		dialer        func(conf zrpc.RpcClientConf) zrpc.Client
 	}
@@ -51,8 +53,24 @@ func (s *Server) Start() {
 }
 
 // Stop stops the gateway server.
+// To get a graceful shutdown, it stops the HTTP server first, then closes gRPC connections.
 func (s *Server) Stop() {
+	// stop the HTTP server first, then close gRPC connections.
+	// in case the gRPC server is stopped first,
+	// the HTTP server may still be running to accept requests.
 	s.Server.Stop()
+
+	group := threading.NewRoutineGroup()
+	for _, conn := range s.conns {
+		// new variable to avoid closure problems, can be removed after go 1.22
+		// see https://golang.org/doc/faq#closures_and_goroutines
+		conn := conn
+		group.Run(func() {
+			// ignore the error when closing the connection
+			_ = conn.Conn().Close()
+		})
+	}
+	group.Wait()
 }
 
 func (s *Server) build() error {
@@ -71,6 +89,7 @@ func (s *Server) build() error {
 		} else {
 			cli = zrpc.MustNewClient(up.Grpc)
 		}
+		s.conns = append(s.conns, cli)
 
 		source, err := s.createDescriptorSource(cli, up)
 		if err != nil {


Test patch:
diff --git a/core/proc/shutdown_test.go b/core/proc/shutdown_test.go
index 79c7deff..64517f0f 100644
--- a/core/proc/shutdown_test.go
+++ b/core/proc/shutdown_test.go
@@ -3,6 +3,7 @@
 package proc
 
 import (
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -29,6 +30,42 @@ func TestShutdown(t *testing.T) {
 	assert.Equal(t, 3, val)
 }
 
+func TestShutdownWithMultipleServices(t *testing.T) {
+	SetTimeToForceQuit(time.Hour)
+	assert.Equal(t, time.Hour, delayTimeBeforeForceQuit)
+
+	var val int32
+	called1 := AddShutdownListener(func() {
+		atomic.AddInt32(&val, 1)
+	})
+	called2 := AddShutdownListener(func() {
+		atomic.AddInt32(&val, 2)
+	})
+	Shutdown()
+	called1()
+	called2()
+
+	assert.Equal(t, int32(3), atomic.LoadInt32(&val))
+}
+
+func TestWrapUpWithMultipleServices(t *testing.T) {
+	SetTimeToForceQuit(time.Hour)
+	assert.Equal(t, time.Hour, delayTimeBeforeForceQuit)
+
+	var val int32
+	called1 := AddWrapUpListener(func() {
+		atomic.AddInt32(&val, 1)
+	})
+	called2 := AddWrapUpListener(func() {
+		atomic.AddInt32(&val, 2)
+	})
+	WrapUp()
+	called1()
+	called2()
+
+	assert.Equal(t, int32(3), atomic.LoadInt32(&val))
+}
+
 func TestNotifyMoreThanOnce(t *testing.T) {
 	ch := make(chan struct{}, 1)
 
diff --git a/gateway/server_test.go b/gateway/server_test.go
index 74168559..68b56ade 100644
--- a/gateway/server_test.go
+++ b/gateway/server_test.go
@@ -46,7 +46,7 @@ func dialer() func(context.Context, string) (net.Conn, error) {
 func TestMustNewServer(t *testing.T) {
 	var c GatewayConf
 	assert.NoError(t, conf.FillDefault(&c))
-	// avoid popup alert on macos for asking permissions
+	// avoid popup alert on MacOS for asking permissions
 	c.DevServer.Host = "localhost"
 	c.Host = "localhost"
 	c.Port = 18881