ipn/ipnlocal: clean up some of the weird locking (#17802)

* lock released early just to call `b.send` when it can call
  `b.sendToLocked` instead
* `UnlockEarly` called to release the lock before trivially fast
  operations, we can wait for a defer there

Updates #11649

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
pull/17815/head
Andrew Lytvynov 3 weeks ago committed by GitHub
parent 2e265213fd
commit ae3dff15e4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -1533,8 +1533,6 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control
return return
} }
if st.Err != nil { if st.Err != nil {
// The following do not depend on any data for which we need b locked.
unlock.UnlockEarly()
if errors.Is(st.Err, io.EOF) { if errors.Is(st.Err, io.EOF) {
b.logf("[v1] Received error: EOF") b.logf("[v1] Received error: EOF")
return return
@ -1543,7 +1541,7 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control
var uerr controlclient.UserVisibleError var uerr controlclient.UserVisibleError
if errors.As(st.Err, &uerr) { if errors.As(st.Err, &uerr) {
s := uerr.UserVisibleError() s := uerr.UserVisibleError()
b.send(ipn.Notify{ErrMessage: &s}) b.sendToLocked(ipn.Notify{ErrMessage: &s}, allClients)
} }
return return
} }
@ -1743,6 +1741,7 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control
b.health.SetLocalLogConfigHealth(errors.New(msg)) b.health.SetLocalLogConfigHealth(errors.New(msg))
// Connecting to this tailnet without logging is forbidden; boot us outta here. // Connecting to this tailnet without logging is forbidden; boot us outta here.
b.mu.Lock() b.mu.Lock()
defer b.mu.Unlock()
// Get the current prefs again, since we unlocked above. // Get the current prefs again, since we unlocked above.
prefs := b.pm.CurrentPrefs().AsStruct() prefs := b.pm.CurrentPrefs().AsStruct()
prefs.WantRunning = false prefs.WantRunning = false
@ -1754,8 +1753,7 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control
}); err != nil { }); err != nil {
b.logf("Failed to save new controlclient state: %v", err) b.logf("Failed to save new controlclient state: %v", err)
} }
b.mu.Unlock() b.sendToLocked(ipn.Notify{ErrMessage: &msg, Prefs: &p}, allClients)
b.send(ipn.Notify{ErrMessage: &msg, Prefs: &p})
return return
} }
if oldNetMap != nil { if oldNetMap != nil {
@ -4795,8 +4793,8 @@ func (b *LocalBackend) setPortlistServices(sl []tailcfg.Service) {
// TODO(danderson): we shouldn't be mangling hostinfo here after // TODO(danderson): we shouldn't be mangling hostinfo here after
// painstakingly constructing it in twelvety other places. // painstakingly constructing it in twelvety other places.
func (b *LocalBackend) doSetHostinfoFilterServices() { func (b *LocalBackend) doSetHostinfoFilterServices() {
unlock := b.lockAndGetUnlock() b.mu.Lock()
defer unlock() defer b.mu.Unlock()
cc := b.cc cc := b.cc
if cc == nil { if cc == nil {
@ -4821,8 +4819,6 @@ func (b *LocalBackend) doSetHostinfoFilterServices() {
hi.Services = []tailcfg.Service{} hi.Services = []tailcfg.Service{}
} }
unlock.UnlockEarly()
// Don't mutate hi.Service's underlying array. Append to // Don't mutate hi.Service's underlying array. Append to
// the slice with no free capacity. // the slice with no free capacity.
c := len(hi.Services) c := len(hi.Services)

Loading…
Cancel
Save