fix: stop local relay teardown from logging a spurious proxy_failed reason

Session.Close now closing its own accepted conn (from the prior fix wave)
made net.ErrClosed on the guacd-side reader indistinguishable from a real
remote failure, so a normal browser-tab close could race the handler's
defer and intermittently log console.proxy_failed on a healthy session.
Add a closing flag, set before Close's sync.Once body actually tears
anything down, that setReason respects -- a deliberate local teardown can
no longer produce or race in a failure reason, while Close's own explicit
reason argument still wins normally.
This commit is contained in:
2026-07-31 09:25:27 +01:00
parent 1fe608f531
commit 978b665aa6
+25 -11
View File
@@ -32,10 +32,11 @@ type Session struct {
allowed []string
timeout time.Duration
once sync.Once
mu sync.Mutex
reason string
conn net.Conn
once sync.Once
mu sync.Mutex
reason string
conn net.Conn
closing bool
rendezvous *time.Timer
}
@@ -71,30 +72,43 @@ func (s *Session) Reason() string {
return s.reason
}
// setReason records r as the session's failure reason, first write wins. It is
// a no-op once a deliberate teardown (Close) has begun: a local Close closing
// the conn out from under the relay goroutines produces exactly the kind of
// error (net.ErrClosed, a broken pipe on write, ...) that looks like a remote
// failure but is not one, and must not overwrite — or race to set — the real
// reason, or invent one where a clean local close has none.
func (s *Session) setReason(r string) {
s.mu.Lock()
if s.reason == "" {
if s.reason == "" && !s.closing {
s.reason = r
}
s.mu.Unlock()
}
// Close tears the session down once. A non-empty reason is recorded only if no
// reason has been recorded already. It closes both the listener and, if a
// connection has already been accepted, that connection too — an unconditional
// kill for the whole relay chain regardless of which stage it is in.
// reason has been recorded already, and only before teardown begins. It closes
// both the listener and, if a connection has already been accepted, that
// connection too — an unconditional kill for the whole relay chain regardless
// of which stage it is in.
func (s *Session) Close(reason string) {
if reason != "" {
s.setReason(reason)
}
s.once.Do(func() {
// Recorded before anything is actually closed: the reader/writer
// goroutines in relay() call setReason from the errors this Close
// itself is about to cause (a closed conn, a closed stream), and
// those must be recognised as teardown noise, not a genuine failure.
s.mu.Lock()
s.closing = true
conn := s.conn
s.mu.Unlock()
if s.rendezvous != nil {
s.rendezvous.Stop()
}
_ = s.listener.Close()
s.mu.Lock()
conn := s.conn
s.mu.Unlock()
if conn != nil {
_ = conn.Close()
}