Skip to content

Commit

Permalink
ipn/ipnlocal: hold the mutex when in onTailnetDefaultAutoUpdate (tail…
Browse files Browse the repository at this point in the history
…scale#11786)

Turns out, profileManager is not safe for concurrent use and I missed
all the locking infrastructure in LocalBackend, oops.

I was not able to reproduce the race even with `go test -count 100`, but
this seems like an obvious fix.

Fixes tailscale#11773

Signed-off-by: Andrew Lytvynov <awly@tailscale.com>
  • Loading branch information
awly committed Apr 18, 2024
1 parent 88a7767 commit 22bd506
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions ipn/ipnlocal/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -2498,6 +2498,9 @@ func (b *LocalBackend) onClientVersion(v *tailcfg.ClientVersion) {
}

func (b *LocalBackend) onTailnetDefaultAutoUpdate(au bool) {
unlock := b.lockAndGetUnlock()
defer unlock()

prefs := b.pm.CurrentPrefs()
if !prefs.Valid() {
b.logf("[unexpected]: received tailnet default auto-update callback but current prefs are nil")
Expand All @@ -2511,12 +2514,12 @@ func (b *LocalBackend) onTailnetDefaultAutoUpdate(au bool) {
b.logf("using tailnet default auto-update setting: %v", au)
prefsClone := prefs.AsStruct()
prefsClone.AutoUpdate.Apply = opt.NewBool(au)
_, err := b.EditPrefs(&ipn.MaskedPrefs{
_, err := b.editPrefsLockedOnEntry(&ipn.MaskedPrefs{
Prefs: *prefsClone,
AutoUpdateSet: ipn.AutoUpdatePrefsMask{
ApplySet: true,
},
})
}, unlock)
if err != nil {
b.logf("failed to apply tailnet-wide default for auto-updates (%v): %v", au, err)
return
Expand Down

0 comments on commit 22bd506

Please sign in to comment.