Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[client] Fix race conditions #2869

Merged
merged 5 commits into from
Nov 11, 2024
Merged

[client] Fix race conditions #2869

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4a6261b
Fix concurrent map access in status
lixmal Nov 8, 2024
7cc3122
Fix race when retrieving ctx state error
lixmal Nov 8, 2024
f19022f
Fix race when accessing service controller server instance
lixmal Nov 8, 2024
0a1501e
Merge branch 'main' into fix-races
lixmal Nov 11, 2024
e28e5ec
Remove code from merge conflict
lixmal Nov 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions client/cmd/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cmd

import (
"context"
"sync"

"github.com/kardianos/service"
log "github.com/sirupsen/logrus"
Expand All @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions client/cmd/service_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -72,13 +74,15 @@ 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)
if err != nil {
log.Errorf("failed to stop daemon: %v", err)
}
}
p.serverInstanceMu.Unlock()

p.cancel()

Expand Down
3 changes: 2 additions & 1 deletion client/internal/connect.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion client/internal/peer/status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -237,6 +237,11 @@ func (d *Status) UpdatePeerState(receivedState State) error {
peerState.IP = receivedState.IP
}

receivedRoutes := receivedState.GetRoutes()
if receivedRoutes != nil {
peerState.SetRoutes(receivedRoutes)
}

skipNotification := shouldSkipNotify(receivedState.ConnStatus, peerState)

if receivedState.ConnStatus != peerState.ConnStatus {
Expand Down
Loading