diff options
| author | Jeff Halter <868228+jhalter@users.noreply.github.com> | 2026-06-12 08:11:55 -0700 |
|---|---|---|
| committer | Jeff Halter <868228+jhalter@users.noreply.github.com> | 2026-06-12 08:11:55 -0700 |
| commit | 7ebc802d0a269218f05b3b51eb10ac66eacb4d1f (patch) | |
| tree | 0c700edcde0fcc034605dfcf98b81ddb1cf174b6 /hotline/server_test.go | |
| parent | d2791fcbadcf332dc05e5bebc7350eca50617263 (diff) | |
Replace shared outbox with per-client send queues
The outbox channel spawned one goroutine per outbound transaction, so
concurrent sends to the same client could interleave bytes within the
transaction framing, per-client message ordering was not guaranteed,
and a slow client accumulated unbounded goroutines.
Each ClientConn now has a bounded send queue drained by a single writer
goroutine, which serializes writes and preserves enqueue order. Send
never blocks: if a client's queue overflows, its connection is closed
and the read loop performs the usual disconnect cleanup. Server.Send
routes transactions to the target client's queue, replacing
processOutbox and sendTransaction. Handler signatures are unchanged.
Disconnect now removes the client from the manager before notifying
peers so no new transactions are routed to a departing client, then
idempotently closes its send queue.
New tests cover write ordering, framing integrity under concurrent
senders, the slow-client disconnect policy, and a Send/Disconnect race
exercise (run with -race).
Diffstat (limited to 'hotline/server_test.go')
| -rw-r--r-- | hotline/server_test.go | 52 |
1 files changed, 22 insertions, 30 deletions
diff --git a/hotline/server_test.go b/hotline/server_test.go index bebde8f..b3f4dce 100644 --- a/hotline/server_test.go +++ b/hotline/server_test.go @@ -764,66 +764,58 @@ func TestServer_CurrentStats(t *testing.T) { assert.Equal(t, 0, result.UploadsInProgress) } -func TestServer_sendTransaction(t *testing.T) { - t.Run("sends transaction to client connection", func(t *testing.T) { - wBuf := &bytes.Buffer{} +func TestServer_Send(t *testing.T) { + t.Run("enqueues transaction for the target client", func(t *testing.T) { + client := &ClientConn{ + Connection: &nopCloserRWC{Buffer: &bytes.Buffer{}}, + } mockMgr := &MockClientMgr{} - mockMgr.On("Get", ClientID{0, 1}).Return(&ClientConn{ - Connection: &nopCloserRWC{Buffer: wBuf}, - }) + mockMgr.On("Get", ClientID{0, 1}).Return(client) srv := &Server{ClientMgr: mockMgr} tran := NewTransaction(TranChatMsg, ClientID{0, 1}, NewField(FieldData, []byte("hello"))) - err := srv.sendTransaction(tran) + srv.Send(tran) - assert.NoError(t, err) - assert.Greater(t, wBuf.Len(), 0) + assert.Len(t, client.sendCh, 1) + queued := <-client.sendCh + assert.Equal(t, TranChatMsg, queued.Type) mockMgr.AssertExpectations(t) }) - t.Run("returns nil when client not found", func(t *testing.T) { + t.Run("drops transaction when client not found", func(t *testing.T) { mockMgr := &MockClientMgr{} mockMgr.On("Get", ClientID{0, 99}).Return((*ClientConn)(nil)) srv := &Server{ClientMgr: mockMgr} - tran := NewTransaction(TranChatMsg, ClientID{0, 99}) - err := srv.sendTransaction(tran) + srv.Send(NewTransaction(TranChatMsg, ClientID{0, 99})) - assert.NoError(t, err) mockMgr.AssertExpectations(t) }) } func TestServer_SendAll(t *testing.T) { - mockMgr := &MockClientMgr{} - mockMgr.On("List").Return([]*ClientConn{ + peers := []*ClientConn{ {ID: ClientID{0, 1}}, {ID: ClientID{0, 2}}, {ID: ClientID{0, 3}}, - }) - - outbox := make(chan Transaction, 10) - srv := &Server{ - ClientMgr: mockMgr, - outbox: outbox, } + mockMgr := &MockClientMgr{} + mockMgr.On("List").Return(peers) + + srv := &Server{ClientMgr: mockMgr} srv.SendAll(TranChatMsg, NewField(FieldData, []byte("broadcast"))) - assert.Len(t, outbox, 3) + // Verify each transaction was queued for its own client + for _, peer := range peers { + assert.Len(t, peer.sendCh, 1) - // Verify each transaction targets a different client - clientIDs := make(map[ClientID]bool) - for range 3 { - tran := <-outbox - clientIDs[tran.ClientID] = true + tran := <-peer.sendCh + assert.Equal(t, peer.ID, tran.ClientID) assert.Equal(t, TranChatMsg, tran.Type) } - assert.True(t, clientIDs[ClientID{0, 1}]) - assert.True(t, clientIDs[ClientID{0, 2}]) - assert.True(t, clientIDs[ClientID{0, 3}]) mockMgr.AssertExpectations(t) } |