From 1139bff62cd350a041623bc4cf97155895f7ce6b Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 8 Nov 2022 13:50:34 -0500 Subject: [PATCH] server: Ensure GRPCServer GracefulStop() and Stop() call GRPCBroker Close() (#220) Initializing a `GRPCServer` will do the following: ```go // Register the broker service brokerServer := newGRPCBrokerServer() plugin.RegisterGRPCBrokerServer(s.server, brokerServer) s.broker = newGRPCBroker(brokerServer, s.TLS) go s.broker.Run() ``` While the current `GracefulStop()` and `Stop()` methods omit any handling of the underlying `s.broker`, which means the broker goroutine will leak: ```text [Goroutine 53 in state select, with github.com/hashicorp/go-plugin.(*gRPCBrokerServer).Recv on top of the stack: goroutine 53 [select]: github.com/hashicorp/go-plugin.(*gRPCBrokerServer).Recv(0x29?) /Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_broker.go:121 +0x58 github.com/hashicorp/go-plugin.(*GRPCBroker).Run(0x140003ca190) /Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_broker.go:411 +0x40 created by github.com/hashicorp/go-plugin.(*GRPCServer).Init /Users/bflad/go/pkg/mod/github.com/hashicorp/go-plugin@v1.4.4/grpc_server.go:85 +0x424 ] ``` This change will now call the `Close()` method on the broker and set it to `nil` when `GracefulStop()` or `Stop()` is called on the server itself. --- grpc_server.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/grpc_server.go b/grpc_server.go index 387628bf..54b061cc 100644 --- a/grpc_server.go +++ b/grpc_server.go @@ -107,14 +107,26 @@ func (s *GRPCServer) Init() error { return nil } -// Stop calls Stop on the underlying grpc.Server +// Stop calls Stop on the underlying grpc.Server and Close on the underlying +// grpc.Broker if present. func (s *GRPCServer) Stop() { s.server.Stop() + + if s.broker != nil { + s.broker.Close() + s.broker = nil + } } -// GracefulStop calls GracefulStop on the underlying grpc.Server +// GracefulStop calls GracefulStop on the underlying grpc.Server and Close on +// the underlying grpc.Broker if present. func (s *GRPCServer) GracefulStop() { s.server.GracefulStop() + + if s.broker != nil { + s.broker.Close() + s.broker = nil + } } // Config is the GRPCServerConfig encoded as JSON then base64.