ipn/ipnserver: close a small race in ipnserver, ~simplify code

There was a small window in ipnserver after we assigned a LocalBackend
to the ipnserver's atomic but before we Start'ed it where our
initalization Start could conflict with API calls from the LocalAPI.

Simplify that a bit and lay out the rules in the docs.

Updates #12028

Change-Id: Ic5f5e4861e26340599184e20e308e709edec68b1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
pull/12053/head
Brad Fitzpatrick 6 months ago
parent 32bc596062
commit 364fad3749

@ -35,6 +35,7 @@ import (
"tailscale.com/control/controlclient"
"tailscale.com/drive/driveimpl"
"tailscale.com/envknob"
"tailscale.com/ipn"
"tailscale.com/ipn/conffile"
"tailscale.com/ipn/ipnlocal"
"tailscale.com/ipn/ipnserver"
@ -480,6 +481,15 @@ func startIPNServer(ctx context.Context, logf logger.Logf, logID logid.PublicID,
lb, err := getLocalBackend(ctx, logf, logID, sys)
if err == nil {
logf("got LocalBackend in %v", time.Since(t0).Round(time.Millisecond))
if lb.Prefs().Valid() {
if err := lb.Start(ipn.Options{}); err != nil {
logf("LocalBackend.Start: %v", err)
lb.Shutdown()
lbErr.Store(err)
cancel()
return
}
}
srv.SetLocalBackend(lb)
close(wgEngineCreated)
return

@ -46,9 +46,6 @@ type Server struct {
// is true, the ForceDaemon pref can override this.
resetOnZero bool
startBackendOnce sync.Once
runCalled atomic.Bool
// mu guards the fields that follow.
// lock order: mu, then LocalBackend.mu
mu sync.Mutex
@ -471,16 +468,15 @@ func New(logf logger.Logf, logID logid.PublicID, netMon *netmon.Monitor) *Server
// SetLocalBackend sets the server's LocalBackend.
//
// If b.Run has already been called, then lb.Start will be called.
// Otherwise Start will be called once Run is called.
// It should only call be called after calling lb.Start.
func (s *Server) SetLocalBackend(lb *ipnlocal.LocalBackend) {
if lb == nil {
panic("nil LocalBackend")
}
if !s.lb.CompareAndSwap(nil, lb) {
panic("already set")
}
s.startBackendIfNeeded()
s.mu.Lock()
s.backendWaiter.wakeAll()
@ -490,21 +486,6 @@ func (s *Server) SetLocalBackend(lb *ipnlocal.LocalBackend) {
// https://github.com/tailscale/tailscale/issues/6522
}
func (b *Server) startBackendIfNeeded() {
if !b.runCalled.Load() {
return
}
lb := b.lb.Load()
if lb == nil {
return
}
if lb.Prefs().Valid() {
b.startBackendOnce.Do(func() {
lb.Start(ipn.Options{})
})
}
}
// connIdentityContextKey is the http.Request.Context's context.Value key for either an
// *ipnauth.ConnIdentity or an error.
type connIdentityContextKey struct{}
@ -517,7 +498,6 @@ type connIdentityContextKey struct{}
// If the Server's LocalBackend has already been set, Run starts it.
// Otherwise, the next call to SetLocalBackend will start it.
func (s *Server) Run(ctx context.Context, ln net.Listener) error {
s.runCalled.Store(true)
defer func() {
if lb := s.lb.Load(); lb != nil {
lb.Shutdown()
@ -537,7 +517,6 @@ func (s *Server) Run(ctx context.Context, ln net.Listener) error {
ln.Close()
}()
s.startBackendIfNeeded()
systemd.Ready()
hs := &http.Server{

Loading…
Cancel
Save