fix(games): reject duplicate queue entries (#117)
Some checks are pending
CI / build (push) Waiting to run
deploy / deploy (push) Waiting to run
test / test (push) Waiting to run

* test(games): reproduce duplicate queue deadlock

* fix(games): reject duplicate queue entries
This commit is contained in:
RissRIce 2026-08-12 22:20:18 -06:00 committed by GitHub
parent be75744248
commit 5a1f5d90db
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 56 additions and 2 deletions

View file

@ -13,6 +13,9 @@ var ErrNoOpponent = errors.New("no opponent found")
// ErrUnknownGame means the requested game id is not in the registry.
var ErrUnknownGame = errors.New("unknown game")
// ErrAlreadyQueued means the same player already has a connection waiting for this game.
var ErrAlreadyQueued = errors.New("player already queued")
// Store persists finished matches and tracks per-game ELO ratings. The SQLite
// store implements it; the matchmaker stays storage-agnostic.
type Store interface {
@ -77,7 +80,11 @@ func (mm *Matchmaker) Play(ctx context.Context, gameID string, io PlayerIO) erro
}
mm.mu.Lock()
if w, waiting := mm.queue[gameID]; waiting && w.io.Name() != io.Name() {
if w, waiting := mm.queue[gameID]; waiting {
if w.io.Name() == io.Name() {
mm.mu.Unlock()
return ErrAlreadyQueued
}
// An opponent is waiting — pair up and run the match.
delete(mm.queue, gameID)
mm.mu.Unlock()

View file

@ -2,6 +2,7 @@ package games
import (
"context"
"errors"
"math"
"sync"
"testing"
@ -133,3 +134,42 @@ func TestMatchmakerUnknownGame(t *testing.T) {
t.Fatalf("want ErrUnknownGame, got %v", err)
}
}
func TestMatchmakerRejectsDuplicateWaiter(t *testing.T) {
mm := NewMatchmaker(Catalog(), nil, time.Second, time.Minute)
firstCtx, cancelFirst := context.WithCancel(context.Background())
firstDone := make(chan error, 1)
go func() {
firstDone <- mm.Play(firstCtx, "ttt", &firstLegalPlayer{name: "same-agent"})
}()
deadline := time.Now().Add(time.Second)
for {
mm.mu.Lock()
queued := mm.queue["ttt"] != nil
mm.mu.Unlock()
if queued {
break
}
if time.Now().After(deadline) {
t.Fatal("first player never entered the queue")
}
time.Sleep(time.Millisecond)
}
duplicateCtx, cancelDuplicate := context.WithTimeout(context.Background(), 50*time.Millisecond)
defer cancelDuplicate()
if err := mm.Play(duplicateCtx, "ttt", &firstLegalPlayer{name: "same-agent"}); err == nil || errors.Is(err, context.DeadlineExceeded) {
t.Fatalf("duplicate Play error = %v, want immediate already-queued rejection", err)
}
cancelFirst()
select {
case err := <-firstDone:
if !errors.Is(err, context.Canceled) {
t.Fatalf("first Play error = %v, want context.Canceled", err)
}
case <-time.After(100 * time.Millisecond):
t.Fatal("first player remained blocked after its context was canceled")
}
}