From e0bed2b0fb29c4abbc6d46191a7f54de90e29122 Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Mon, 11 Nov 2024 14:55:10 +0100 Subject: [PATCH] [client] Fix race conditions (#2869) * Fix concurrent map access in status * Fix race when retrieving ctx state error * Fix race when accessing service controller server instance --- client/cmd/service.go | 10 ++++++---- client/cmd/service_controller.go | 4 ++++ client/internal/connect.go | 3 ++- client/internal/peer/status.go | 2 +- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/client/cmd/service.go b/client/cmd/service.go index 855eb30fa77..3560088a7f1 100644 --- a/client/cmd/service.go +++ b/client/cmd/service.go @@ -2,6 +2,7 @@ package cmd import ( "context" + "sync" "github.com/kardianos/service" log "github.com/sirupsen/logrus" @@ -13,10 +14,11 @@ import ( ) type program struct { - ctx context.Context - cancel context.CancelFunc - serv *grpc.Server - serverInstance *server.Server + ctx context.Context + cancel context.CancelFunc + serv *grpc.Server + serverInstance *server.Server + serverInstanceMu sync.Mutex } func newProgram(ctx context.Context, cancel context.CancelFunc) *program { diff --git a/client/cmd/service_controller.go b/client/cmd/service_controller.go index 86546e31c9e..761c8662834 100644 --- a/client/cmd/service_controller.go +++ b/client/cmd/service_controller.go @@ -61,7 +61,9 @@ func (p *program) Start(svc service.Service) error { } proto.RegisterDaemonServiceServer(p.serv, serverInstance) + p.serverInstanceMu.Lock() p.serverInstance = serverInstance + p.serverInstanceMu.Unlock() log.Printf("started daemon server: %v", split[1]) if err := p.serv.Serve(listen); err != nil { @@ -72,6 +74,7 @@ func (p *program) Start(svc service.Service) error { } func (p *program) Stop(srv service.Service) error { + p.serverInstanceMu.Lock() if p.serverInstance != nil { in := new(proto.DownRequest) _, err := p.serverInstance.Down(p.ctx, in) @@ -79,6 +82,7 @@ func (p *program) Stop(srv service.Service) error { log.Errorf("failed to stop daemon: %v", err) } } + p.serverInstanceMu.Unlock() p.cancel() diff --git a/client/internal/connect.go b/client/internal/connect.go index bcc9d17a3f6..dff44f1d2ce 100644 --- a/client/internal/connect.go +++ b/client/internal/connect.go @@ -207,7 +207,8 @@ func (c *ConnectClient) run(mobileDependency MobileDependency, probes *ProbeHold c.statusRecorder.MarkSignalDisconnected(nil) defer func() { - c.statusRecorder.MarkSignalDisconnected(state.err) + _, err := state.Status() + c.statusRecorder.MarkSignalDisconnected(err) }() // with the global Wiretrustee config in hand connect (just a connection, no stream yet) Signal diff --git a/client/internal/peer/status.go b/client/internal/peer/status.go index 241dfabbb01..0444dc60b1d 100644 --- a/client/internal/peer/status.go +++ b/client/internal/peer/status.go @@ -67,7 +67,7 @@ func (s *State) DeleteRoute(network string) { func (s *State) GetRoutes() map[string]struct{} { s.Mux.RLock() defer s.Mux.RUnlock() - return s.routes + return maps.Clone(s.routes) } // LocalPeerState contains the latest state of the local peer