Skip to content

Commit

Permalink
don't double-close when NetAppender uses shared connection (viamrobot…
Browse files Browse the repository at this point in the history
  • Loading branch information
abe-winter authored Jun 10, 2024
1 parent ca9330f commit 1d2dd98
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 2 deletions.
10 changes: 8 additions & 2 deletions logging/net_appender.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ func NewNetAppender(config *CloudConfig, conn rpc.ClientConn) (*NetAppender, err
}

logWriter := &remoteLogWriterGRPC{
cfg: config,
rpcClient: conn,
cfg: config,
rpcClient: conn,
clientIsShared: conn != nil,
}
if conn != nil {
logWriter.service = apppb.NewRobotServiceClient(conn)
Expand Down Expand Up @@ -307,6 +308,8 @@ type remoteLogWriterGRPC struct {
service apppb.RobotServiceClient
rpcClient rpc.ClientConn
clientMutex sync.Mutex
// when clientIsShared = true, don't close it in close(); it's externally managed.
clientIsShared bool
}

func (w *remoteLogWriterGRPC) write(logs []*commonpb.LogEntry) error {
Expand Down Expand Up @@ -347,6 +350,9 @@ func (w *remoteLogWriterGRPC) getOrCreateClient(ctx context.Context) (apppb.Robo
}

func (w *remoteLogWriterGRPC) close() {
if w.clientIsShared {
return
}
w.clientMutex.Lock()
defer w.clientMutex.Unlock()

Expand Down
1 change: 1 addition & 0 deletions logging/net_appender_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ func TestProvidedClientConn(t *testing.T) {
defer server.stop()
conn, err := CreateNewGRPCClient(context.Background(), server.cloudConfig)
test.That(t, err, test.ShouldBeNil)
defer conn.Close()
netAppender, err := NewNetAppender(server.cloudConfig, conn)
test.That(t, err, test.ShouldBeNil)
// make sure these are the same object, i.e. that the constructor set it properly.
Expand Down

0 comments on commit 1d2dd98

Please sign in to comment.