From 35ddd0ad2215760bb1d3dc7724480f179ea33fa6 Mon Sep 17 00:00:00 2001 From: historia <[not public]> Date: Wed, 16 Sep 2026 21:05:39 -0400 Subject: fix: login account race, casino chip double-refund, room-create rollback, rename collision, fatal transport errors --- internal/net/server.go | 68 +++++++++++++++++++++++++++++++++- internal/net/transport_failure_test.go | 44 ++++++++++++++++++++++ 2 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 internal/net/transport_failure_test.go (limited to 'internal/net') diff --git a/internal/net/server.go b/internal/net/server.go index eed5486..66d31e3 100644 --- a/internal/net/server.go +++ b/internal/net/server.go @@ -81,6 +81,14 @@ type Session struct { disconnecting atomic.Bool disconnectTicks atomic.Int64 + // authName carries the account name for the password-entry state. The + // owner goroutine sets it when offering the password prompt, while the + // connection goroutine reads it for the off-owner password + // verification — so unlike Session.Account (which ONLY the owner may + // touch, and which account republish can replace concurrently), it is + // an atomic: a plain field read off the owner is a data race. + authName atomic.Pointer[string] + // Outbound queueing. Transport sessions (telnet/TLS/websocket) are put // into queued mode by startWriter before their input goroutine begins: // Write* then enqueues into outbox, and the dedicated writer goroutine @@ -108,6 +116,21 @@ type Session struct { seq uint64 } +// SetAuthName records the account name offered for password entry. +// Owner goroutine only. +func (sess *Session) SetAuthName(name string) { + sess.authName.Store(&name) +} + +// AuthName returns the account name recorded for password entry, or "" +// when none was recorded. Safe on the connection goroutine. +func (sess *Session) AuthName() string { + if p := sess.authName.Load(); p != nil { + return *p + } + return "" +} + // sessionSeq is the source of Session.seq values. It is incremented under the // adding Hub's mutex, so no extra atomicity is needed for correctness; atomic // access keeps the package-level default counter safe if Hub.Add is ever @@ -126,7 +149,15 @@ type Server struct { done chan struct{} shutdownOnce sync.Once - listenErr error + // finishOnce guards the single close of done: both the graceful + // shutdown path and a fatal transport failure (a listener that cannot + // bind, an Accept loop that dies) converge on finish, so ListenAndServe + // unblocks either way and done is never closed twice. + finishOnce sync.Once + listenErr error + // stopping flips when Shutdown begins so listener goroutines treat + // their subsequent Accept/serve errors as expected, not fatal. + stopping atomic.Bool } type Hub struct { @@ -511,11 +542,27 @@ func (s *Server) ListenAndServe(handler func(*Session, string)) error { return s.listenErr } +// finish records the terminal transport error (nil for a clean shutdown) +// exactly once and unblocks ListenAndServe. Fatal transport failures — a +// listener that cannot bind or an Accept loop that dies outside shutdown — +// reach it too, so a misconfigured server does not sit running with no way +// in. +func (s *Server) finish(err error) { + s.finishOnce.Do(func() { + s.listenErr = err + close(s.done) + }) +} + // Shutdown stops all transports: HTTP(S) servers drain, listeners close, and // every session is closed (its outbound queue drains first). ListenAndServe // returns after Shutdown completes. It is safe to call more than once. func (s *Server) Shutdown() { s.shutdownOnce.Do(func() { + // Before touching any listener: goroutines serving those listeners + // check this flag so their resulting errors are not reported as + // fatal. + s.stopping.Store(true) if s.httpServer != nil { ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) defer cancel() @@ -535,7 +582,7 @@ func (s *Server) Shutdown() { for _, sess := range s.hub.AllSessions() { sess.Close() } - close(s.done) + s.finish(nil) }) <-s.done } @@ -544,6 +591,10 @@ func (s *Server) serveTelnet() { for { conn, err := s.telnetLn.Accept() if err != nil { + if !s.stopping.Load() { + log.Printf("telnet listener error: %v", err) + s.finish(fmt.Errorf("telnet listener: %w", err)) + } return } sess := &Session{ @@ -563,6 +614,10 @@ func (s *Server) serveTelnetTLS() { for { conn, err := s.telnetTLSLn.Accept() if err != nil { + if !s.stopping.Load() { + log.Printf("telnet+tls listener error: %v", err) + s.finish(fmt.Errorf("telnet+tls listener: %w", err)) + } return } sess := &Session{ @@ -579,6 +634,9 @@ func (s *Server) serveHTTP() { err := s.httpServer.ListenAndServe() if err != nil && err != http.ErrServerClosed { log.Printf("http server error: %v", err) + if !s.stopping.Load() { + s.finish(fmt.Errorf("http server: %w", err)) + } } } @@ -587,11 +645,17 @@ func (s *Server) serveHTTPS() { ln, err := net.Listen("tcp", addr) if err != nil { log.Printf("https server error: %v", err) + if !s.stopping.Load() { + s.finish(fmt.Errorf("https listen: %w", err)) + } return } tlsLn := tls.NewListener(ln, s.httpsTLSConfig) if err := s.httpsServer.Serve(tlsLn); err != nil && err != http.ErrServerClosed { log.Printf("https server error: %v", err) + if !s.stopping.Load() { + s.finish(fmt.Errorf("https server: %w", err)) + } } } diff --git a/internal/net/transport_failure_test.go b/internal/net/transport_failure_test.go new file mode 100644 index 0000000..ecd41e8 --- /dev/null +++ b/internal/net/transport_failure_test.go @@ -0,0 +1,44 @@ +package net + +// Regression: a fatal transport failure (a listener that cannot bind, an +// Accept loop that dies outside shutdown) must unblock ListenAndServe with +// the error — a server must never sit running with no way in — and a +// failure followed by Shutdown must not panic on a double close of done. + +import ( + "fmt" + "net" + "net/http" + "testing" + "time" +) + +func TestTransportFailureStopsListenAndServe(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + port := fmt.Sprintf("127.0.0.1:%d", ln.Addr().(*net.TCPAddr).Port) + + s := &Server{hub: NewHub(), done: make(chan struct{})} + s.httpServer = &http.Server{Addr: port, Handler: http.NewServeMux()} + + result := make(chan error, 1) + go func() { + go s.serveHTTP() + <-s.done + result <- s.listenErr + }() + + select { + case err := <-result: + if err == nil { + t.Fatal("transport failure unblocked ListenAndServe without an error") + } + // Shutdown after a failure must not double-close done. + s.Shutdown() + case <-time.After(2 * time.Second): + t.Fatal("a failed listener left the server running") + } +} -- cgit v1.2.3