mirror of
https://github.com/tailscale/tailscale.git
synced 2025-02-18 02:48:40 +00:00
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>
This commit is contained in:
parent
32bc596062
commit
727c0d6cfd
@ -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…
x
Reference in New Issue
Block a user