Fix closing already closed channel on exit

This commit is contained in:
bdnugget 2025-04-16 12:22:08 +02:00
parent 9d60d5e9cd
commit 62a6bb2926
2 changed files with 83 additions and 28 deletions

View File

@ -410,7 +410,13 @@ func (g *Game) Cleanup() {
rl.UnloadMusicStream(g.AssetManager.Music) rl.UnloadMusicStream(g.AssetManager.Music)
} }
close(g.quitChan) // Only close the channel if it hasn't been closed yet
select {
case <-g.quitChan:
// Channel already closed, do nothing
default:
close(g.quitChan)
}
}) })
} }
@ -491,7 +497,8 @@ func (g *Game) DrawMenu() {
} }
func (g *Game) Shutdown() { func (g *Game) Shutdown() {
close(g.quitChan) // Use the cleanup method which has channel-closing safety
g.Cleanup()
} }
func (g *Game) HandleServerMessages(messages []*pb.ChatMessage) { func (g *Game) HandleServerMessages(messages []*pb.ChatMessage) {

View File

@ -248,23 +248,43 @@ func ConnectToServer(username, password string, isRegistering bool) (net.Conn, i
func HandleServerCommunication(conn net.Conn, playerID int32, player *types.Player, otherPlayers map[int32]*types.Player, quitChan <-chan struct{}) { func HandleServerCommunication(conn net.Conn, playerID int32, player *types.Player, otherPlayers map[int32]*types.Player, quitChan <-chan struct{}) {
msgHandler := NewMessageHandler(conn) msgHandler := NewMessageHandler(conn)
// Create channels for coordinating goroutines
errChan := make(chan error, 1)
done := make(chan struct{})
// Create a WaitGroup to track both sender and receiver goroutines
var wg sync.WaitGroup
wg.Add(2) // One for sender, one for receiver
// Set up a deferred cleanup function
defer func() { defer func() {
if r := recover(); r != nil { if r := recover(); r != nil {
log.Printf("Recovered from panic in HandleServerCommunication: %v", r) log.Printf("Recovered from panic in HandleServerCommunication: %v", r)
} }
// Close the done channel to signal both goroutines to exit
close(done)
// Wait for both goroutines to finish
wg.Wait()
// Close the connection
conn.Close() conn.Close()
// Close the player's QuitDone channel if it exists
if player.QuitDone != nil { if player.QuitDone != nil {
close(player.QuitDone) select {
case <-player.QuitDone: // Check if it's already closed
// Already closed, do nothing
default:
close(player.QuitDone)
}
} }
}() }()
actionTicker := time.NewTicker(types.ClientTickRate) actionTicker := time.NewTicker(types.ClientTickRate)
defer actionTicker.Stop() defer actionTicker.Stop()
// Create error channel for goroutine communication
errChan := make(chan error, 1)
done := make(chan struct{})
// Add a heartbeat ticker to detect connection issues // Add a heartbeat ticker to detect connection issues
heartbeatTicker := time.NewTicker(5 * time.Second) heartbeatTicker := time.NewTicker(5 * time.Second)
defer heartbeatTicker.Stop() defer heartbeatTicker.Stop()
@ -276,8 +296,14 @@ func HandleServerCommunication(conn net.Conn, playerID int32, player *types.Play
defer func() { defer func() {
if r := recover(); r != nil { if r := recover(); r != nil {
log.Printf("Recovered from panic in message sender: %v", r) log.Printf("Recovered from panic in message sender: %v", r)
errChan <- fmt.Errorf("message sender panic: %v", r) select {
case errChan <- fmt.Errorf("message sender panic: %v", r):
default:
// Channel already closed or full, just log
log.Printf("Unable to send error: %v", r)
}
} }
wg.Done() // Mark this goroutine as done
}() }()
for { for {
@ -291,8 +317,11 @@ func HandleServerCommunication(conn net.Conn, playerID int32, player *types.Play
PlayerId: playerID, PlayerId: playerID,
}}, }},
} }
msgHandler.WriteMessage(disconnectMsg)
done <- struct{}{} // Try to send disconnect message, ignoring errors
_ = msgHandler.WriteMessage(disconnectMsg)
// No need to signal done channel here, the main goroutine handles this
return return
case <-done: case <-done:
return return
@ -307,8 +336,11 @@ func HandleServerCommunication(conn net.Conn, playerID int32, player *types.Play
} }
if err := msgHandler.WriteMessage(emptyBatch); err != nil { if err := msgHandler.WriteMessage(emptyBatch); err != nil {
log.Printf("Failed to send heartbeat: %v", err) log.Printf("Failed to send heartbeat: %v", err)
errChan <- err select {
return case errChan <- err:
case <-done:
return
}
} }
lastMessageTime = time.Now() lastMessageTime = time.Now()
} }
@ -327,8 +359,11 @@ func HandleServerCommunication(conn net.Conn, playerID int32, player *types.Play
player.Unlock() player.Unlock()
if err := msgHandler.WriteMessage(batch); err != nil { if err := msgHandler.WriteMessage(batch); err != nil {
errChan <- err select {
return case errChan <- err:
case <-done:
return
}
} }
lastMessageTime = time.Now() lastMessageTime = time.Now()
} else { } else {
@ -343,14 +378,22 @@ func HandleServerCommunication(conn net.Conn, playerID int32, player *types.Play
defer func() { defer func() {
if r := recover(); r != nil { if r := recover(); r != nil {
log.Printf("Recovered from panic in message receiver: %v", r) log.Printf("Recovered from panic in message receiver: %v", r)
errChan <- fmt.Errorf("message receiver panic: %v", r) select {
case errChan <- fmt.Errorf("message receiver panic: %v", r):
default:
// Channel already closed or full, just log
log.Printf("Unable to send error: %v", r)
}
} }
wg.Done() // Mark this goroutine as done
}() }()
for { for {
select { select {
case <-quitChan: case <-quitChan:
return return
case <-done:
return
default: default:
serverMessage, err := msgHandler.ReadMessage() serverMessage, err := msgHandler.ReadMessage()
if err != nil { if err != nil {
@ -358,7 +401,11 @@ func HandleServerCommunication(conn net.Conn, playerID int32, player *types.Play
log.Printf("Network timeout: %v", err) log.Printf("Network timeout: %v", err)
} else if err != io.EOF { } else if err != io.EOF {
log.Printf("Network read error: %v", err) log.Printf("Network read error: %v", err)
errChan <- err select {
case errChan <- err:
case <-done:
return
}
} else { } else {
log.Printf("Connection closed by server") log.Printf("Connection closed by server")
} }
@ -375,19 +422,12 @@ func HandleServerCommunication(conn net.Conn, playerID int32, player *types.Play
select { select {
case <-quitChan: case <-quitChan:
log.Printf("Received quit signal, sending disconnect message") log.Printf("Received quit signal, sending disconnect message")
// Send disconnect message // The cleanup will happen in the deferred function
disconnectMsg := &pb.ActionBatch{ return
PlayerId: playerID,
Actions: []*pb.Action{{
Type: pb.Action_DISCONNECT,
PlayerId: playerID,
}},
}
msgHandler.WriteMessage(disconnectMsg)
close(done)
case err := <-errChan: case err := <-errChan:
log.Printf("Network error: %v", err) log.Printf("Network error: %v", err)
close(done) // The cleanup will happen in the deferred function
return
} }
} }
@ -420,7 +460,13 @@ func NewConnection(username, password string, isRegistering bool) (*Connection,
func (c *Connection) Close() { func (c *Connection) Close() {
c.closeOnce.Do(func() { c.closeOnce.Do(func() {
close(c.quitChan) select {
case <-c.quitChan: // Check if it's already closed
// Already closed, do nothing
default:
close(c.quitChan)
}
// Wait with timeout for network cleanup // Wait with timeout for network cleanup
select { select {
case <-c.quitDone: case <-c.quitDone:
@ -428,6 +474,8 @@ func (c *Connection) Close() {
case <-time.After(500 * time.Millisecond): case <-time.After(500 * time.Millisecond):
log.Println("Network cleanup timed out") log.Println("Network cleanup timed out")
} }
// Make sure the connection is closed
c.conn.Close() c.conn.Close()
}) })
} }