Share one VNC session agent manager across all accept loops

client/vnc/server/agent_ipc.go

@@ -24,16 +24,6 @@ import (
 // browser can show a meaningful message.
 var errNoConsoleUser = errors.New("no user logged into console")
 
-// sessionAgent abstracts the per-platform manager that spawns and tracks
-// the user-session VNC agent. Resolve returns the agent's Unix-socket
-// path, the shared per-spawn token, and the uid the agent was spawned
-// under (used to validate peer credentials before the daemon hands the
-// token to whoever is on the other end of the socket). Resolve may spawn
-// the agent lazily.
-type sessionAgent interface {
-	Resolve(ctx context.Context) (socketPath, token string, peerUID uint32, err error)
-}
-
 // prefixConn replays already-consumed header bytes ahead of the proxy
 // stream by swapping in a different Reader on the same underlying Conn.
 type prefixConn struct {

client/vnc/server/server.go

@@ -183,8 +183,15 @@ type Server struct {
 	ctx            context.Context
 	cancel         context.CancelFunc
 	vmgr           virtualSessionManager
-	authorizer     *sshauth.Authorizer
-	netstackNet    *netstack.Net
+	// serviceAgentMu guards the shared per-session agent manager below, which
+	// every service-mode accept loop resolves through; see Server.serviceAgent.
+	// Its own mutex rather than mu: Stop holds mu while tearing it down.
+	serviceAgentMu      sync.Mutex
+	serviceAgentMgr     sessionAgent
+	serviceAgentStop    func()
+	serviceAgentStopped bool
+	authorizer          *sshauth.Authorizer
+	netstackNet         *netstack.Net
 	// agentToken holds the raw token bytes for agent-mode auth.
 	agentToken []byte
 	// invalidAgentToken latches when AgentTokenHex was provided but failed
@@ -710,6 +717,8 @@ func (s *Server) Stop() error {
 		s.vmgr.StopAll()
 	}
 
+	s.stopServiceAgent()
+
 	if s.serviceMode {
 		s.platformShutdown()
 	}

client/vnc/server/server_darwin.go

@@ -29,8 +29,11 @@ func (s *Server) serviceAcceptLoop(ln net.Listener) {
 		return
 	}
 
-	mgr := newDarwinAgentManager(s.ctx)
-	defer mgr.stop()
+	mgr := s.serviceAgent()
+	if mgr == nil {
+		s.log.Error("service mode: no agent manager available")
+		return
+	}
 
 	log.Info("service mode, proxying connections to per-user agent over Unix socket")
 
@@ -61,3 +64,10 @@ func (s *Server) serviceAcceptLoop(ln net.Listener) {
 		}(conn)
 	}
 }
+
+// newServiceAgentManager starts the manager that owns the per-user agent.
+// Called once per server via Server.serviceAgent.
+func (s *Server) newServiceAgentManager() (sessionAgent, func()) {
+	mgr := newDarwinAgentManager(s.ctx)
+	return mgr, mgr.stop
+}

client/vnc/server/server_windows.go

@@ -246,8 +246,11 @@ func (s *Server) serviceAcceptLoop(ln net.Listener) {
 		return
 	}
 
-	sm := newSessionManager()
-	go sm.run()
+	sm := s.serviceAgent()
+	if sm == nil {
+		s.log.Error("service mode: no agent manager available")
+		return
+	}
 
 	log.Info("service mode, proxying connections to agent over Unix socket")
 
@@ -256,7 +259,6 @@ func (s *Server) serviceAcceptLoop(ln net.Listener) {
 		if err != nil {
 			select {
 			case <-s.ctx.Done():
-				sm.Stop()
 				return
 			default:
 			}
@@ -279,3 +281,11 @@ func (s *Server) serviceAcceptLoop(ln net.Listener) {
 		}(conn)
 	}
 }
+
+// newServiceAgentManager starts the session manager that owns the console
+// agent. Called once per server via Server.serviceAgent.
+func (s *Server) newServiceAgentManager() (sessionAgent, func()) {
+	sm := newSessionManager()
+	go sm.run()
+	return sm, sm.Stop
+}

client/vnc/server/server_x11.go

@@ -21,3 +21,9 @@ func (s *Server) platformSessionManager() virtualSessionManager {
 func (s *Server) platformShutdown() {
 	// no-op on this platform
 }
+
+// newServiceAgentManager has no service mode to manage on this platform;
+// serviceAcceptLoop falls back to direct mode and never asks for it.
+func (s *Server) newServiceAgentManager() (sessionAgent, func()) {
+	return nil, nil
+}

client/vnc/server/service_agent.go

@@ -0,0 +1,56 @@
+package server
+
+import "context"
+
+// sessionAgent abstracts the per-platform manager that spawns and tracks
+// the user-session VNC agent. Resolve returns the agent's Unix-socket
+// path, the shared per-spawn token, and the uid the agent was spawned
+// under (used to validate peer credentials before the daemon hands the
+// token to whoever is on the other end of the socket). Resolve may spawn
+// the agent lazily.
+type sessionAgent interface {
+	Resolve(ctx context.Context) (socketPath, token string, peerUID uint32, err error)
+}
+
+// serviceAgent returns the one agent manager this server shares across every
+// service-mode accept loop, constructing it on first use.
+//
+// One per server, not one per listener: the manager owns the agent process for
+// the active session, so a second manager spawns a second agent on its own
+// socket path, and each accept loop then proxies to a different one. Only one of
+// those agents ends up serving, so connections arriving on the other listener
+// are refused with the agent socket actively refusing the dial. A dual-stack
+// server has two accept loops, since the v6 overlay listener is added after
+// Start, which is how that happened.
+//
+// Returns nil once the server has stopped, and on platforms with no service
+// mode.
+func (s *Server) serviceAgent() sessionAgent {
+	s.serviceAgentMu.Lock()
+	defer s.serviceAgentMu.Unlock()
+
+	if s.serviceAgentStopped {
+		return nil
+	}
+	if s.serviceAgentMgr == nil {
+		s.serviceAgentMgr, s.serviceAgentStop = s.newServiceAgentManager()
+	}
+	return s.serviceAgentMgr
+}
+
+// stopServiceAgent tears down the shared manager, if one was ever built, and
+// latches the server so a still-draining accept loop cannot build another.
+// Owned by Stop rather than by an accept loop: the loops share the manager, so
+// the first one to exit must not take it away from the others.
+func (s *Server) stopServiceAgent() {
+	s.serviceAgentMu.Lock()
+	stop := s.serviceAgentStop
+	s.serviceAgentStop = nil
+	s.serviceAgentMgr = nil
+	s.serviceAgentStopped = true
+	s.serviceAgentMu.Unlock()
+
+	if stop != nil {
+		stop()
+	}
+}