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 855eb30fa..3560088a7 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 86546e31c..761c86628 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 bcc9d17a3..dff44f1d2 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 241dfabbb..0444dc60b 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