fix

client/pool_test.go

@@ -0,0 +1,112 @@
+package client
+
+import (
+	"net"
+	"sync"
+	"testing"
+	"time"
+)
+
+// stalledHub accepts connections and then says nothing: it never answers the
+// Rekey frame with SessionReady, and never closes. This models a hub with a
+// wedged event loop, or a load balancer accepting on behalf of a dead backend.
+func stalledHub(t *testing.T) string {
+	t.Helper()
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	var mu sync.Mutex
+	var held []net.Conn
+	t.Cleanup(func() {
+		_ = ln.Close()
+		mu.Lock()
+		for _, c := range held {
+			_ = c.Close()
+		}
+		mu.Unlock()
+	})
+	go func() {
+		for {
+			c, err := ln.Accept()
+			if err != nil {
+				return
+			}
+			mu.Lock()
+			held = append(held, c)
+			mu.Unlock()
+		}
+	}()
+	return ln.Addr().String()
+}
+
+// TestAllocateDoesNotWedgePoolOnStalledHub is the regression guard for the
+// worst failure mode found in the stability audit: Allocate used to dial while
+// holding the pool mutex, and the handshake read had no deadline. One
+// unresponsive hub therefore parked every present and future allocation
+// forever, so no player could be served again until the process restarted.
+func TestAllocateDoesNotWedgePoolOnStalledHub(t *testing.T) {
+	c := New(&Config{
+		Server:         stalledHub(t),
+		PSK:            "pool-test",
+		MaxConn:        8,
+		PingIntervalMs: 20000,
+		Mappings:       []Mapping{{Pattern: "mc.local", Destination: "127.0.0.1:1"}},
+	})
+
+	done := make(chan error, 2)
+	go func() { _, _, err := c.pool.Allocate(); done <- err }()
+	time.Sleep(200 * time.Millisecond) // let the first caller get into the dial
+	go func() { _, _, err := c.pool.Allocate(); done <- err }()
+
+	// Both must give up on their own; neither may be stuck behind the other.
+	limit := time.After(HandshakeTimeout + 15*time.Second)
+	for i := 0; i < 2; i++ {
+		select {
+		case err := <-done:
+			if err == nil {
+				t.Fatal("Allocate succeeded against a hub that never answers")
+			}
+		case <-limit:
+			t.Fatalf("Allocate #%d never returned: the pool is wedged again", i+1)
+		}
+	}
+}
+
+// TestAllocateSpreadsAcrossConns guards the allocation rule: the pool must fan
+// out to maxConn before stacking streams, so a single worker connection is
+// never the shared point of failure for every player. Seven players used to all
+// land on one conn, which meant one dead TCP connection dropped everybody.
+func TestAllocateSpreadsAcrossConns(t *testing.T) {
+	p := &WorkerPool{maxConn: 4}
+	p.cond = sync.NewCond(&p.mu)
+
+	newConn := func() *WorkerConn {
+		return &WorkerConn{pool: p, streams: make(map[int]*Stream), nextSid: 1, done: make(chan struct{})}
+	}
+	p.conns = []*WorkerConn{newConn()}
+	p.conns[0].registerStream(1, &Stream{sid: 1})
+
+	// One conn holding a stream, pool below maxConn: growth is warranted.
+	_, bestCount := p.leastLoadedLocked()
+	if bestCount < StreamsBeforeGrowing {
+		t.Fatalf("a conn with %d stream(s) should trigger growth", bestCount)
+	}
+
+	// Once the pool is at maxConn, growth stops and streams stack on the
+	// least-loaded conn instead.
+	for len(p.conns) < p.maxConn {
+		p.conns = append(p.conns, newConn())
+	}
+	best, bestCount := p.leastLoadedLocked()
+	if bestCount != 0 {
+		t.Fatalf("expected an empty conn to be least-loaded, got %d streams", bestCount)
+	}
+	p.maybeGrowLocked(bestCount)
+	if p.dialing != 0 {
+		t.Fatalf("pool dialed past maxConn=%d", p.maxConn)
+	}
+	if best == nil {
+		t.Fatal("no conn selected")
+	}
+}