Skip to content

Commit

Permalink
Merge pull request #193 from joshzcold/fix/pint_interval_panic_v2
Browse files Browse the repository at this point in the history
fix(backend) Fix potential send to closed channel from pingInterval
  • Loading branch information
joshzcold authored Feb 23, 2025
2 parents e09dc35 + bdb60ff commit 4e1d908
Show file tree
Hide file tree
Showing 6 changed files with 7 additions and 15 deletions.
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ e2e: dev-background
e2e-ui: dev-background
npm install
cd e2e
npm install
npx playwright test --ui
cd -
$(MAKE) dev-down
Expand Down
1 change: 0 additions & 1 deletion backend/api/buzzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ func RegisterBuzzer(client *Client, event *Event) GameError {
return GameError{code: PLAYER_NOT_FOUND}
}
// Set up recurring ping loop to get player latency
clientPlayer.stopPing = make(chan bool)
go clientPlayer.pingInterval()
s.writeRoom(room.Game.Room, room)
return GameError{}
Expand Down
3 changes: 0 additions & 3 deletions backend/api/hub.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ func (h *Hub) run() {
defer func() {
for client := range h.clients {
client.conn.Close()
close(client.send)
}
close(h.register)
close(h.unregister)
Expand All @@ -49,14 +48,12 @@ func (h *Hub) run() {
case client := <-h.unregister:
if _, ok := h.clients[client]; ok {
delete(h.clients, client)
close(client.send)
}
case message := <-h.broadcast:
for client := range h.clients {
select {
case client.send <- message:
default:
close(client.send)
delete(h.clients, client)
}
}
Expand Down
4 changes: 2 additions & 2 deletions backend/api/room.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ func (p *RegisteredClient) pingInterval() error {
if p.client.send != nil {
p.client.send <- message
}
case <-p.stopPing:
// Stop ping interval when client stops
case <-p.client.stop:
log.Println("Stop ping via channel", p.id)
return nil
}
Expand Down Expand Up @@ -102,7 +103,6 @@ type RegisteredClient struct {
id string
client *Client
room *room
stopPing chan bool
}

type roomConnections struct {
Expand Down
9 changes: 2 additions & 7 deletions backend/api/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ func quitPlayer(room *room, client *Client, event *Event) error {
}
}

if ok && playerClient.stopPing != nil {
// clear interval
playerClient.stopPing <- true
}
message, err := NewSendQuit()
if err != nil {
return fmt.Errorf(" %w", err)
Expand Down Expand Up @@ -177,15 +173,14 @@ func getBackInPlayer(client *Client, room room, roomCode string, playerID string

playerClient, ok := room.registeredClients[playerID]
if ok {
if playerClient.stopPing != nil {
playerClient.stopPing <- true
if playerClient.client.stop != nil {
playerClient.client.stop <- true
}
}
playerClient = &RegisteredClient{
id: playerID,
client: client,
room: &room,
stopPing: make(chan bool),
}
go playerClient.pingInterval()
room.registeredClients[playerID] = playerClient
Expand Down
4 changes: 3 additions & 1 deletion e2e/tests/game.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ test("quit button should return to home page", async ({ browser }) => {
const host = await s.host();
const spectator = await s.addPlayer(true);
const gamePage = new GamePage(spectator.page);
await gamePage.quitButton.click();
await expect(async () => {
await gamePage.quitButton.click();
}).toPass();
await expect(host.page).toHaveURL("/");
});

0 comments on commit 4e1d908

Please sign in to comment.