Skip to content

Commit

Permalink
server: Ensure GRPCServer GracefulStop() and Stop() call GRPCBroker C…
Browse files Browse the repository at this point in the history
…lose() (#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.
  • Loading branch information
bflad authored Nov 8, 2022
1 parent 118b62a commit 1139bff
Showing 1 changed file with 14 additions and 2 deletions.
16 changes: 14 additions & 2 deletions grpc_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 1139bff

Please sign in to comment.