diff --git a/client/internal/connect.go b/client/internal/connect.go index 7e444ad49..c6fc940df 100644 --- a/client/internal/connect.go +++ b/client/internal/connect.go @@ -472,26 +472,19 @@ func parseRelayInfo(loginResp *mgmProto.LoginResponse) ([]string, *hmac.Token) { return relayCfg.GetUrls(), token } -// IsRunning reports whether a client run is currently in flight. It is the -// single source of truth for "is the client running", answered by the -// supervisor via a serialized query (so it settles behind an in-flight stop). -// Intended for the external status surface (CLI/UI), not internal pre-checks. -func (c *ConnectClient) IsRunning() bool { - if c == nil || c.sup == nil { - return false - } +// ConnectionRunning reports whether a connection run is currently in flight +// (connecting, connected, or reconnecting). Answered by the supervisor via a +// serialized query, so it settles behind an in-flight stop. Distinct from +// ServiceRunning, which reports whether the service itself is alive. +func (c *ConnectClient) ConnectionRunning() bool { return c.sup.isRunning() } // ServiceRunning reports whether the client's lifecycle supervisor is alive and -// able to accept start/stop commands — i.e. the daemon-lifetime client exists -// and its context has not been cancelled. It is independent of whether a run is -// currently up (that is IsRunning). Nil-safe, so callers can ask it on a -// not-yet-constructed client and treat false as "service not running". +// able to accept start/stop commands — i.e. its context has not been cancelled +// (the daemon is not shutting down). Independent of whether a connection run is +// up (that is ConnectionRunning). func (c *ConnectClient) ServiceRunning() bool { - if c == nil || c.sup == nil { - return false - } return c.sup.ctx.Err() == nil } diff --git a/client/server/mdm.go b/client/server/mdm.go index 16ea489c8..78734ea84 100644 --- a/client/server/mdm.go +++ b/client/server/mdm.go @@ -38,12 +38,11 @@ type conflictCheck struct { // OS-native managed-config store reports a diff vs the last observation. // // Restart sequence: -// 1. Cancel the active engine context (terminates connectWithRetryRuns). -// 2. Wait briefly for that goroutine to exit (giveUpChan is closed on exit). -// 3. Re-resolve Config from disk + MDM policy (Config.apply re-runs +// 1. Stop the in-flight run via the supervisor (blocks until fully torn down). +// 2. Re-resolve Config from disk + MDM policy (Config.apply re-runs // applyMDMPolicy with the freshly loaded Policy). -// 4. Spawn a fresh connectWithRetryRuns with the new context and config. -// 5. Broadcast a SystemEvent so any GUI / CLI subscriber (SubscribeEvents +// 3. Start a fresh run with the new config. +// 4. Broadcast a SystemEvent so any GUI / CLI subscriber (SubscribeEvents // RPC) can refresh its cached config view without polling. // // The callback runs in the ticker's own goroutine. Ticker has already @@ -51,17 +50,13 @@ type conflictCheck struct { func (s *Server) onMDMPolicyChange(_, _ *mdm.Policy) error { log.Warn("MDM policy changed; restarting engine to apply new configuration") - // Hold s.mutex for the entire restart sequence (cancel + quiescence - // wait + re-spawn). Any concurrent Up/Down/Status arriving while - // MDM is restarting blocks on the Lock until we are done — they - // then observe the post-restart state coherently. This is safe - // because the connectWithRetryRuns goroutine no longer acquires - // s.mutex in its defer (intent vs. goroutine-alive concerns are - // fully separated; see the connectionGoroutineRunning helper). + // Hold s.mutex for the entire restart sequence (stop + re-start). Any + // concurrent Up/Down/Status arriving while MDM is restarting blocks on the + // Lock until we are done — they then observe the post-restart state coherently. s.mutex.Lock() defer s.mutex.Unlock() - if !s.connectClient.IsRunning() { + if !s.connectClient.ConnectionRunning() { // No run in flight, so there's no engine to restart. return nil } @@ -123,14 +118,13 @@ func (s *Server) publishConfigChangedEvent(source string) { } // restartEngineForMDMLocked re-resolves the active profile config -// (re-running applyMDMPolicy via Config.apply) and re-spawns -// connectWithRetryRuns. Mirrors the tail of Server.Start so a runtime -// MDM change behaves identically to a fresh boot under the new policy. +// (re-running applyMDMPolicy via Config.apply) and starts a fresh run. +// Mirrors the tail of Server.Start so a runtime MDM change behaves +// identically to a fresh boot under the new policy. // // MUST be called with s.mutex held — onMDMPolicyChange holds the lock -// for the entire restart sequence (cancel + quiescence wait + re-spawn) -// so concurrent Up/Down/Status RPCs observe a coherent post-restart -// state. +// for the entire restart sequence so concurrent Up/Down/Status RPCs +// observe a coherent post-restart state. func (s *Server) restartEngineForMDMLocked() error { activeProf, err := s.profileManager.GetActiveProfileState() if err != nil { diff --git a/client/server/server.go b/client/server/server.go index 4eeedcdf1..30c46e4ef 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -63,7 +63,7 @@ type Server struct { config *profilemanager.Config proto.UnimplementedDaemonServiceServer // Whether a run is in flight is owned by the supervisor - // (connectClient.IsRunning); the daemon keeps no separate "running" flag. + // (connectClient.ConnectionRunning); the daemon keeps no separate flag. clientRunningChan chan struct{} // closed by the run when the engine is ready clientDoneChan chan error // receives the run's end result @@ -139,7 +139,7 @@ func (s *Server) Start() error { s.mutex.Lock() defer s.mutex.Unlock() - if s.connectClient.IsRunning() { + if s.connectClient.ConnectionRunning() { return nil } @@ -665,11 +665,9 @@ func (s *Server) Up(callerCtx context.Context, msg *proto.UpRequest) (*proto.UpR return nil, fmt.Errorf("service is not running, start the netbird service for 'up' to take effect") } - // clientRunning is the daemon-intent flag (set by previous Up/Start, cleared - // by Down). IsRunning() reports whether a run is actually in flight. When - // intent is up AND a run is in flight, the existing engine is on the job — - // just wait for it. Otherwise fall through to start a fresh run. - if s.connectClient.IsRunning() { + // If a connection run is already in flight, the existing engine is on the + // job — just wait for it. Otherwise fall through to start a fresh run. + if s.connectClient.ConnectionRunning() { state := internal.CtxGetState(s.rootCtx) status, err := state.Status() if err != nil { @@ -1029,7 +1027,7 @@ func (s *Server) validateProfileOperation(profileName string, allowActiveProfile // logoutFromProfile logs out from a specific profile by loading its config and sending logout request func (s *Server) logoutFromProfile(ctx context.Context, profileName, username string) error { activeProf, err := s.profileManager.GetActiveProfileState() - if err == nil && activeProf.Name == profileName && s.connectClient.IsRunning() { + if err == nil && activeProf.Name == profileName && s.connectClient.ConnectionRunning() { return s.sendLogoutRequest(ctx) } @@ -1087,7 +1085,7 @@ func (s *Server) Status( runningChan := s.clientRunningChan doneChan := s.clientDoneChan s.mutex.Unlock() - alive := client != nil && client.IsRunning() + alive := client.ConnectionRunning() if msg.WaitForReady != nil && *msg.WaitForReady && alive { select { @@ -1335,7 +1333,7 @@ func (s *Server) WaitJWTToken( // ExposeService exposes a local port via the NetBird reverse proxy. func (s *Server) ExposeService(req *proto.ExposeServiceRequest, srv proto.DaemonService_ExposeServiceServer) error { s.mutex.Lock() - if !s.connectClient.IsRunning() { + if !s.connectClient.ConnectionRunning() { s.mutex.Unlock() return gstatus.Errorf(codes.FailedPrecondition, "client is not running, run 'netbird up' first") }