mirror of
https://github.com/tiennm99/noitu.git
synced 2026-10-14 16:17:44 +00:00
fix(wsapi): guard room seating races, refuse games during drain, and drop the history copy
A session can tear down between the hub handing a room its seating message and the room goroutine draining it off the queue. Nothing else ever learns that, since leaveRoom only notifies a room the session had already attached to. Left seated as connected, allConnected() could report true and let quick match auto-start a game against a dead socket. handleCreate, handleJoin, handleStartBot and handleResume now check the connection's context and either reopen the grace window the disconnect would have, or cancel the room when there is no lobby left to hold one open. beginGame could also raise the live-game count after the drain decision: an existing lobby's StartGame, and quick match's own auto-start, now refuse with server_restarting once the hub is draining, matching the refusal newRegisteredRoom already gives a brand-new room. Engine.Snapshot() copied the whole move history on every broadcast for two callers that only ever wanted the last move or the played-word set. Engine.LastMove and Engine.UsedWords answer both directly off the engine's own state, State.History is gone, and Standings is only computed once the game is actually over, which is the only time it is meaningful.
This commit is contained in:
1 parent
eae8d42f25
commit
8b3e8f7e67
6 files changed
+211
-27
No files matched your search
@@ -10,6 +10,8 @@ package game
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"iter"
|
||||
"maps"
|
||||
"math/bits"
|
||||
"slices"
|
||||
"time"
|
||||
@@ -179,6 +181,25 @@ func (e *Engine) Winner() PlayerID { return e.winner }
|
||||
// ChainLength reports how many words have been played, opening word included.
|
||||
func (e *Engine) ChainLength() int { return len(e.history) + 1 }
|
||||
|
||||
// LastMove reports the most recently played word, and false when none has
|
||||
// been played yet. It exists so a caller that only ever wants the tail of the
|
||||
// chain — the resume replay is the one — does not have to copy the whole
|
||||
// history to reach it.
|
||||
func (e *Engine) LastMove() (Move, bool) {
|
||||
if len(e.history) == 0 {
|
||||
return Move{}, false
|
||||
}
|
||||
return e.history[len(e.history)-1], true
|
||||
}
|
||||
|
||||
// UsedWords iterates every canonical word already played, the opening word
|
||||
// included. It is read directly off the engine's own set rather than rebuilt
|
||||
// from history on every call, which is what a bot's board was doing once per
|
||||
// move.
|
||||
func (e *Engine) UsedWords() iter.Seq[string] {
|
||||
return maps.Keys(e.used)
|
||||
}
|
||||
|
||||
// Submit validates a player's word and, if legal, plays it.
|
||||
//
|
||||
// The returned Move carries the canonical spelling; on rejection the reason
|
||||
@@ -575,11 +596,19 @@ func (e *Engine) Snapshot() State {
|
||||
alive[p] = e.alive[i]
|
||||
}
|
||||
|
||||
// Standings is meaningless while the game is in play, by its own doc
|
||||
// comment, so it is only worth computing once the game actually has one —
|
||||
// otherwise every TurnUpdate and every bot move pays for a table nobody
|
||||
// reads.
|
||||
var standings []Standing
|
||||
if e.over {
|
||||
standings = e.Standings()
|
||||
}
|
||||
|
||||
return State{
|
||||
Current: e.current,
|
||||
Turn: e.Turn(),
|
||||
Deadline: e.deadline,
|
||||
History: append([]Move{}, e.history...),
|
||||
Scores: scores,
|
||||
Alive: alive,
|
||||
Eliminated: append([]PlayerID{}, e.outOrder...),
|
||||
@@ -587,6 +616,6 @@ func (e *Engine) Snapshot() State {
|
||||
Over: e.over,
|
||||
Winner: e.winner,
|
||||
EndReason: e.endReason,
|
||||
Standings: e.Standings(),
|
||||
Standings: standings,
|
||||
}
|
||||
}
|
||||
@@ -756,18 +756,60 @@ func TestSnapshotIsACopy(t *testing.T) {
|
||||
e.Submit(alice, "ngữ pháp", t0)
|
||||
|
||||
snap := e.Snapshot()
|
||||
snap.History[0].Word = "MUTATED"
|
||||
snap.Scores[alice] = 9999
|
||||
|
||||
fresh := e.Snapshot()
|
||||
if fresh.History[0].Word == "MUTATED" {
|
||||
t.Error("mutating a snapshot's history changed engine state")
|
||||
}
|
||||
if fresh.Scores[alice] == 9999 {
|
||||
t.Error("mutating a snapshot's scores changed engine state")
|
||||
}
|
||||
}
|
||||
|
||||
// LastMove is the resume replay's whole reason to exist: the tail of the
|
||||
// chain without copying the rest of it.
|
||||
func TestLastMove(t *testing.T) {
|
||||
e := newGame(t, standardDict(), "ngôn ngữ")
|
||||
|
||||
if _, ok := e.LastMove(); ok {
|
||||
t.Fatal("LastMove reported a move before any word was played")
|
||||
}
|
||||
|
||||
if _, r := e.Submit(alice, "ngữ pháp", t0); r != ReasonNone {
|
||||
t.Fatalf("Submit: %s", r)
|
||||
}
|
||||
move, ok := e.LastMove()
|
||||
if !ok {
|
||||
t.Fatal("LastMove reported none after a word was played")
|
||||
}
|
||||
if move.Word != "ngữ pháp" {
|
||||
t.Errorf("LastMove = %q, want %q", move.Word, "ngữ pháp")
|
||||
}
|
||||
|
||||
if _, r := e.Submit(bob, "pháp luật", t0); r != ReasonNone {
|
||||
t.Fatalf("Submit: %s", r)
|
||||
}
|
||||
move, ok = e.LastMove()
|
||||
if !ok || move.Word != "pháp luật" {
|
||||
t.Errorf("LastMove after a second word = %+v, %v, want %q", move, ok, "pháp luật")
|
||||
}
|
||||
}
|
||||
|
||||
// UsedWords is the bot's search boundary: the opening word and everything
|
||||
// played since, with nothing else in it.
|
||||
func TestUsedWords(t *testing.T) {
|
||||
e := newGame(t, standardDict(), "ngôn ngữ")
|
||||
if _, r := e.Submit(alice, "ngữ pháp", t0); r != ReasonNone {
|
||||
t.Fatalf("Submit: %s", r)
|
||||
}
|
||||
|
||||
used := map[string]bool{}
|
||||
for w := range e.UsedWords() {
|
||||
used[w] = true
|
||||
}
|
||||
if !used["ngôn ngữ"] || !used["ngữ pháp"] || len(used) != 2 {
|
||||
t.Errorf("UsedWords = %v, want exactly the opening word and the move played", used)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRejectReasonStrings(t *testing.T) {
|
||||
// Every reason needs a distinct, non-empty description: these become
|
||||
// player-facing messages.
|
||||
|
||||
@@ -212,8 +212,8 @@ func TestAnEliminatedPlayerKeepsTheirScore(t *testing.T) {
|
||||
if got := e.Score(alice); got != scored {
|
||||
t.Errorf("score after elimination = %d, want %d", got, scored)
|
||||
}
|
||||
if state := e.Snapshot(); len(state.History) != 1 || state.History[0].Player != alice {
|
||||
t.Errorf("the eliminated player's move is missing from the history: %+v", state.History)
|
||||
if last, ok := e.LastMove(); !ok || last.Player != alice {
|
||||
t.Errorf("the eliminated player's move is missing from the history: %+v, %v", last, ok)
|
||||
}
|
||||
if !slices.Contains(e.Players(), alice) {
|
||||
t.Error("Players dropped the eliminated seat, which the transport still has to render")
|
||||
|
||||
@@ -186,7 +186,6 @@ type State struct {
|
||||
Current string
|
||||
Turn PlayerID
|
||||
Deadline time.Time
|
||||
History []Move
|
||||
Scores map[PlayerID]int
|
||||
// Alive says who is still in the game. A player who has been eliminated
|
||||
// keeps their score and their place in the history; they simply no longer
|
||||
|
||||
@@ -92,6 +92,59 @@ func TestLiveGameCountTracksGamesNotLobbies(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestDrainRefusesStartGameOnAnExistingLobby covers what newRegisteredRoom's
|
||||
// own drain check cannot: a lobby that existed before the drain decision has
|
||||
// no further room-creation call to refuse, so beginGame itself has to know.
|
||||
func TestDrainRefusesStartGameOnAnExistingLobby(t *testing.T) {
|
||||
api, url := newTestServer(t, chainDict(), Config{})
|
||||
host, guest, _ := pvpLobby(t, url)
|
||||
guest.setReady(true)
|
||||
host.await("room_state")
|
||||
|
||||
api.StartDraining()
|
||||
|
||||
host.startGame()
|
||||
if got := host.await("error").GetError().GetCode(); got != "server_restarting" {
|
||||
t.Errorf("error code = %q, want server_restarting", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestDrainRefusesQuickMatchAutoStart is C3's other beginGame call site: the
|
||||
// second seat filling after the drain decision must not start a game either,
|
||||
// even though nobody sent StartGame for it to refuse.
|
||||
func TestDrainRefusesQuickMatchAutoStart(t *testing.T) {
|
||||
h := &hub{}
|
||||
h.draining.Store(true)
|
||||
first := &session{id: "a", ctx: t.Context(), out: make(chan []byte, 8)}
|
||||
second := &session{id: "b", ctx: t.Context(), out: make(chan []byte, 8)}
|
||||
|
||||
r := &room{hub: h, dict: chainDict(), turnLimit: time.Second, graceFor: time.Minute}
|
||||
r.handleCreate(createInput{sess: first, autoStart: true})
|
||||
r.handleJoin(joinInput{sess: second})
|
||||
|
||||
if r.engine != nil {
|
||||
t.Error("quick match auto-started a game after the drain decision")
|
||||
}
|
||||
}
|
||||
|
||||
// TestDrainLetsALiveGameFinish is the other half of C3: draining must not cut
|
||||
// a game that was already running short, which is the very outcome it exists
|
||||
// to avoid.
|
||||
func TestDrainLetsALiveGameFinish(t *testing.T) {
|
||||
api, url := newTestServer(t, chainDict(), Config{TurnLimit: 200 * time.Millisecond})
|
||||
lead, waits, _ := pvpGame(t, url)
|
||||
|
||||
api.StartDraining()
|
||||
|
||||
// Nobody submits anything: the lead's own turn runs out the clock, which
|
||||
// is enough to end a two-seat game, same as TestLiveGameCountTracksGamesNotLobbies.
|
||||
waits.await("game_over")
|
||||
lead.await("game_over")
|
||||
if got := api.LiveGameCount(); got != 0 {
|
||||
t.Errorf("live games = %d after the game ended during a drain, want 0", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestVersionEndpoint: GET /version answers with exactly the string the
|
||||
// server was configured with, in plain text, so a deploy check can diff it
|
||||
// against what was just built without parsing anything.
|
||||
|
||||
@@ -571,7 +571,8 @@ func (r *room) run() {
|
||||
// The code goes out in the RoomState the run loop broadcasts, so a client can
|
||||
// never be handed a code before the seat behind it exists.
|
||||
func (r *room) handleCreate(m createInput) {
|
||||
r.seats[0] = &seat{id: "p1", nickname: m.sess.nickname(), sess: m.sess, chatFrom: r.chatSeq}
|
||||
s := &seat{id: "p1", nickname: m.sess.nickname(), sess: m.sess, chatFrom: r.chatSeq}
|
||||
r.seats[0] = s
|
||||
r.owner = "p1"
|
||||
r.autoStart = m.autoStart
|
||||
m.sess.attach(r, "p1")
|
||||
@@ -581,10 +582,31 @@ func (r *room) handleCreate(m createInput) {
|
||||
// who was also waiting in it from another attempt — one dequeue serves
|
||||
// both room-entry paths.
|
||||
r.hub.cancelQuickMatch(m.sess)
|
||||
|
||||
if s.sess.ctx.Err() != nil {
|
||||
r.disconnectGhostSeat(s)
|
||||
return
|
||||
}
|
||||
// Deliberately sent to a brand-new room's creator, where it is always
|
||||
// empty: it is what replaces the conversation a client may still be
|
||||
// holding from a room it was in before this one.
|
||||
r.sendChatHistory(r.seats[0])
|
||||
r.sendChatHistory(s)
|
||||
}
|
||||
|
||||
// disconnectGhostSeat opens the seat's reconnect window the moment it is
|
||||
// filled, for a connection that turns out to have already torn down.
|
||||
//
|
||||
// The session can die between the hub handing this room the seating message
|
||||
// and the room goroutine draining it off the queue — nothing else ever learns
|
||||
// that, because leaveRoom only notifies a room the session was already
|
||||
// attached to, and attaching is exactly what has not happened yet. Left
|
||||
// seated as if connected, allConnected() would report true and quick match's
|
||||
// own auto-start (see handleJoin) could begin a game against a socket nobody
|
||||
// is behind. Applying the same grace window handleDisconnect would reuses the
|
||||
// one mechanism that already bounds this instead of adding a second one.
|
||||
func (r *room) disconnectGhostSeat(s *seat) {
|
||||
s.sess = nil
|
||||
s.graceUntil = time.Now().Add(r.graceFor)
|
||||
}
|
||||
|
||||
// handleResign is one player giving up on their own turn. The seat, not the
|
||||
@@ -660,12 +682,24 @@ func (r *room) handleStartBot(m startBotInput) {
|
||||
}
|
||||
|
||||
r.strategy = strategy
|
||||
r.seats[0] = &seat{id: "p1", nickname: m.sess.nickname(), sess: m.sess, chatFrom: r.chatSeq}
|
||||
s := &seat{id: "p1", nickname: m.sess.nickname(), sess: m.sess, chatFrom: r.chatSeq}
|
||||
r.seats[0] = s
|
||||
r.seats[1] = &seat{id: botPlayerID, nickname: "Máy"}
|
||||
r.owner = "p1"
|
||||
m.sess.attach(r, "p1")
|
||||
r.hub.cancelQuickMatch(m.sess)
|
||||
|
||||
if s.sess.ctx.Err() != nil {
|
||||
// A bot room has no lobby to fall back to and no idle timer covering it
|
||||
// while there is no engine yet (resetIdleTimer skips any room with a
|
||||
// strategy) — a grace window here would leave the bot's own seat
|
||||
// holding the room open forever with nothing left to vacate it. The
|
||||
// room ends now instead, the same way a failed bot.New or beginGame
|
||||
// above already does.
|
||||
r.cancel()
|
||||
return
|
||||
}
|
||||
|
||||
if err := r.beginGame(); err != nil {
|
||||
slog.Error("could not start bot game", "room", r.code, "err", err)
|
||||
m.sess.send(errorMsg("game_start_failed"))
|
||||
@@ -703,7 +737,7 @@ func (r *room) handleJoin(m joinInput) {
|
||||
}
|
||||
|
||||
id := seatIDs[free]
|
||||
r.seats[free] = &seat{
|
||||
s := &seat{
|
||||
id: id,
|
||||
nickname: distinguish(m.sess.nickname(), r.takenNicknames(id)),
|
||||
sess: m.sess,
|
||||
@@ -711,10 +745,16 @@ func (r *room) handleJoin(m joinInput) {
|
||||
// read. A room code is pasted into group chats by design.
|
||||
chatFrom: r.chatSeq,
|
||||
}
|
||||
r.seats[free] = s
|
||||
m.sess.attach(r, string(id))
|
||||
r.lobbyChanged = true
|
||||
r.hub.cancelQuickMatch(m.sess)
|
||||
r.sendChatHistory(r.seats[free])
|
||||
|
||||
if s.sess.ctx.Err() != nil {
|
||||
r.disconnectGhostSeat(s)
|
||||
return
|
||||
}
|
||||
r.sendChatHistory(s)
|
||||
|
||||
// A quick match seats both players itself rather than waiting on
|
||||
// readiness and StartGame — there is no owner here to press it, only two
|
||||
@@ -723,6 +763,14 @@ func (r *room) handleJoin(m joinInput) {
|
||||
// beat before GameStarted rather than jumping straight into one with no
|
||||
// seating frame behind it.
|
||||
if r.autoStart && r.seatedCount() >= minPlayers && r.allConnected() {
|
||||
if r.hub.isDraining() {
|
||||
// The second seat filled after the drain decision. There is no
|
||||
// owner here to answer with server_restarting the way lobbyStart
|
||||
// does, so both seats are told directly; the lobby view they are
|
||||
// left in still shows each other, via lobbyChanged below.
|
||||
r.broadcastError("server_restarting")
|
||||
return
|
||||
}
|
||||
r.autoStart = false
|
||||
r.lobbyChanged = false
|
||||
r.broadcastRoomState()
|
||||
@@ -776,6 +824,13 @@ func (r *room) handleLobby(m lobbyInput) {
|
||||
return
|
||||
}
|
||||
switch {
|
||||
case r.hub.isDraining():
|
||||
// newRegisteredRoom already refuses a brand-new room once draining
|
||||
// starts; this lobby existed before that point, and starting its
|
||||
// game now would raise hub.liveGames after the drain decided how
|
||||
// long to wait for exactly that number to reach zero.
|
||||
m.sess.send(errorMsg("server_restarting"))
|
||||
return
|
||||
case r.seatedCount() < minPlayers:
|
||||
m.sess.send(errorMsg("need_more_players"))
|
||||
return
|
||||
@@ -1113,7 +1168,7 @@ func (r *room) maybeScheduleBot() {
|
||||
// resign and disconnect the room processes while the bot thinks — and
|
||||
// bot.Board.Used reads engine state, so the race would be real, not
|
||||
// theoretical.
|
||||
board := freezeBoard(r.engine, r.opening)
|
||||
board := freezeBoard(r.engine)
|
||||
seq := r.turnSeq
|
||||
strategy := r.strategy
|
||||
|
||||
@@ -1472,6 +1527,16 @@ func (r *room) handleResume(m resumeInput) {
|
||||
// somebody who is already back. The run loop sends it to all of them.
|
||||
r.lobbyChanged = true
|
||||
|
||||
if m.sess.ctx.Err() != nil {
|
||||
// The new connection can die between the client's Hello landing and
|
||||
// this resume being drained off the room's queue, the same race
|
||||
// handleCreate and handleJoin guard against. Reopening the window it
|
||||
// just closed leaves the seat exactly as reachable as it was before
|
||||
// this resume was ever attempted.
|
||||
r.disconnectGhostSeat(s)
|
||||
return
|
||||
}
|
||||
|
||||
// Before the lobby return below, not after it: a refresh in the lobby is
|
||||
// the commonest resume there is, and it is exactly the one that would miss
|
||||
// a replay hung off the end of this function.
|
||||
@@ -1484,8 +1549,7 @@ func (r *room) handleResume(m resumeInput) {
|
||||
}
|
||||
state := r.engine.Snapshot()
|
||||
r.sendGameStarted(s, state)
|
||||
if len(state.History) > 0 {
|
||||
last := state.History[len(state.History)-1]
|
||||
if last, ok := r.engine.LastMove(); ok {
|
||||
r.sendTurnUpdate(s, state, &last, r.moveMeanings(&last))
|
||||
}
|
||||
}
|
||||
@@ -1827,16 +1891,13 @@ type frozenBoard struct {
|
||||
dict game.Dictionary
|
||||
}
|
||||
|
||||
func freezeBoard(e *game.Engine, opening string) *frozenBoard {
|
||||
state := e.Snapshot()
|
||||
|
||||
// History omits the opening word, but the engine counts it as played. A
|
||||
// board that disagreed would let the bot pick a word the engine then
|
||||
// rejects as already used.
|
||||
used := make(map[string]struct{}, len(state.History)+1)
|
||||
used[opening] = struct{}{}
|
||||
for _, m := range state.History {
|
||||
used[m.Word] = struct{}{}
|
||||
func freezeBoard(e *game.Engine) *frozenBoard {
|
||||
// UsedWords already includes the opening word — it seeds the engine's own
|
||||
// set — so there is nothing left to add here, and nothing to copy out of a
|
||||
// history that grows with the game.
|
||||
used := make(map[string]struct{})
|
||||
for word := range e.UsedWords() {
|
||||
used[word] = struct{}{}
|
||||
}
|
||||
|
||||
return &frozenBoard{legal: e.LegalMoves(), used: used, dict: e.Dict()}
|
||||
|
||||
Reference in new issue
Block a user