From 8e919b4ee9c43cb195a235d8204371092ef2e116 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Thu, 30 Jul 2026 14:29:34 +0200 Subject: [PATCH] Share one VNC session agent manager across all accept loops --- client/vnc/server/agent_ipc.go | 10 ------ client/vnc/server/server.go | 13 +++++-- client/vnc/server/server_darwin.go | 14 ++++++-- client/vnc/server/server_windows.go | 16 +++++++-- client/vnc/server/server_x11.go | 6 ++++ client/vnc/server/service_agent.go | 56 +++++++++++++++++++++++++++++ 6 files changed, 98 insertions(+), 17 deletions(-) create mode 100644 client/vnc/server/service_agent.go diff --git a/client/vnc/server/agent_ipc.go b/client/vnc/server/agent_ipc.go index 7bfb988de..283241e37 100644 --- a/client/vnc/server/agent_ipc.go +++ b/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 { diff --git a/client/vnc/server/server.go b/client/vnc/server/server.go index 6f4581220..ca63fc33c 100644 --- a/client/vnc/server/server.go +++ b/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() } diff --git a/client/vnc/server/server_darwin.go b/client/vnc/server/server_darwin.go index 1924c3556..055066049 100644 --- a/client/vnc/server/server_darwin.go +++ b/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 +} diff --git a/client/vnc/server/server_windows.go b/client/vnc/server/server_windows.go index 74ef18407..99c548973 100644 --- a/client/vnc/server/server_windows.go +++ b/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 +} diff --git a/client/vnc/server/server_x11.go b/client/vnc/server/server_x11.go index e10846410..dd5ab8e40 100644 --- a/client/vnc/server/server_x11.go +++ b/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 +} diff --git a/client/vnc/server/service_agent.go b/client/vnc/server/service_agent.go new file mode 100644 index 000000000..ca90a07b1 --- /dev/null +++ b/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() + } +}