fix(stage): fix deadlock that was preventing stage change.

This commit is contained in:
Houmgaor
2025-10-27 01:11:57 +01:00
parent 7e9440d8cc
commit 488e8fa045
4 changed files with 69 additions and 13 deletions

View File

@@ -25,6 +25,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Race condition in stage broadcast causing nil pointer panics during player logout - Race condition in stage broadcast causing nil pointer panics during player logout
- Client crash when loading decoration presets (decomyset) with more than 40 entries - Client crash when loading decoration presets (decomyset) with more than 40 entries
- Config file handling and validation
- Fixes 3 critical race conditions in handlers_stage.go
- Fix an issue causing a crash on clans with 0 members
- Fixed deadlock in zone change causing 60-second timeout when players change zones
- Fixed crash when sending empty packets in QueueSend/QueueSendNonBlocking
- Fixed missing stage transfer packet for empty zones
### Security
- Bumped golang.org/x/net from 0.33.0 to 0.38.0
- Bumped golang.org/x/crypto from 0.31.0 to 0.35.0
## [9.2.0] - 2023-04-01 ## [9.2.0] - 2023-04-01

View File

@@ -0,0 +1,15 @@
BEGIN;
-- Initialize otomoairou (mercenary data) with default empty data for characters that have NULL or empty values
-- This prevents error logs when loading mercenary data during zone transitions
UPDATE characters
SET otomoairou = decode(repeat('00', 10), 'hex')
WHERE otomoairou IS NULL OR length(otomoairou) = 0;
-- Initialize platemyset (plate configuration) with default empty data for characters that have NULL or empty values
-- This prevents error logs when loading plate data during zone transitions
UPDATE characters
SET platemyset = decode(repeat('00', 1920), 'hex')
WHERE platemyset IS NULL OR length(platemyset) = 0;
COMMIT;

View File

@@ -97,7 +97,8 @@ func doStageTransfer(s *Session, ackHandle uint32, stageID string) {
s.stage = s.server.stages[stageID] s.stage = s.server.stages[stageID]
s.Unlock() s.Unlock()
// Tell the client to cleanup its current stage objects // Tell the client to cleanup its current stage objects.
// Use blocking send to ensure this critical cleanup packet is not dropped.
s.QueueSendMHF(&mhfpacket.MsgSysCleanupObject{}) s.QueueSendMHF(&mhfpacket.MsgSysCleanupObject{})
// Confirm the stage entry // Confirm the stage entry
@@ -151,10 +152,12 @@ func doStageTransfer(s *Session, ackHandle uint32, stageID string) {
s.stage.RUnlock() s.stage.RUnlock()
} }
// FIX: Always send stage transfer packet, even if empty.
// The client expects this packet to complete the zone change, regardless of content.
// Previously, if newNotif was empty (no users, no objects), no packet was sent,
// causing the client to timeout after 60 seconds.
newNotif.WriteUint16(0x0010) // End it. newNotif.WriteUint16(0x0010) // End it.
if len(newNotif.Data()) > 2 {
s.QueueSend(newNotif.Data()) s.QueueSend(newNotif.Data())
}
} }
func destructEmptyStages(s *Session) { func destructEmptyStages(s *Session) {
@@ -172,17 +175,36 @@ func destructEmptyStages(s *Session) {
} }
func removeSessionFromStage(s *Session) { func removeSessionFromStage(s *Session) {
// Acquire stage lock to protect concurrent access to clients and objects maps
// This prevents race conditions when multiple goroutines access these maps
s.stage.Lock()
// Remove client from old stage. // Remove client from old stage.
delete(s.stage.clients, s) delete(s.stage.clients, s)
// Delete old stage objects owned by the client. // Collect objects to delete while holding lock
s.logger.Info("Sending notification to old stage clients") s.logger.Info("Sending notification to old stage clients")
var objectsToDelete []*Object
for _, object := range s.stage.objects { for _, object := range s.stage.objects {
if object.ownerCharID == s.charID { if object.ownerCharID == s.charID {
s.stage.BroadcastMHF(&mhfpacket.MsgSysDeleteObject{ObjID: object.id}, s) objectsToDelete = append(objectsToDelete, object)
}
}
// Delete from map while still holding lock
for _, object := range objectsToDelete {
delete(s.stage.objects, object.ownerCharID) delete(s.stage.objects, object.ownerCharID)
} }
// CRITICAL FIX: Unlock BEFORE broadcasting to avoid deadlock
// BroadcastMHF also tries to lock the stage, so we must release our lock first
s.stage.Unlock()
// Now broadcast the deletions (without holding the lock)
for _, object := range objectsToDelete {
s.stage.BroadcastMHF(&mhfpacket.MsgSysDeleteObject{ObjID: object.id}, s)
} }
destructEmptyStages(s) destructEmptyStages(s)
destructEmptySemaphores(s) destructEmptySemaphores(s)
} }

View File

@@ -147,7 +147,10 @@ func (s *Session) Start() {
// //
// Thread Safety: Safe for concurrent calls from multiple goroutines. // Thread Safety: Safe for concurrent calls from multiple goroutines.
func (s *Session) QueueSend(data []byte) { func (s *Session) QueueSend(data []byte) {
// FIX: Check data length before reading opcode to prevent crash on empty packets
if len(data) >= 2 {
s.logMessage(binary.BigEndian.Uint16(data[0:2]), data, "Server", s.Name) s.logMessage(binary.BigEndian.Uint16(data[0:2]), data, "Server", s.Name)
}
select { select {
case s.sendPackets <- packet{data, false}: case s.sendPackets <- packet{data, false}:
// Enqueued data // Enqueued data
@@ -182,7 +185,9 @@ func (s *Session) QueueSend(data []byte) {
func (s *Session) QueueSendNonBlocking(data []byte) { func (s *Session) QueueSendNonBlocking(data []byte) {
select { select {
case s.sendPackets <- packet{data, true}: case s.sendPackets <- packet{data, true}:
if len(data) >= 2 {
s.logMessage(binary.BigEndian.Uint16(data[0:2]), data, "Server", s.Name) s.logMessage(binary.BigEndian.Uint16(data[0:2]), data, "Server", s.Name)
}
default: default:
s.logger.Warn("Packet queue too full, dropping!") s.logger.Warn("Packet queue too full, dropping!")
} }
@@ -231,10 +236,13 @@ func (s *Session) sendLoop() {
if s.closed { if s.closed {
return return
} }
// Send each packet individually with its own terminator
for len(s.sendPackets) > 0 {
pkt := <-s.sendPackets pkt := <-s.sendPackets
err := s.cryptConn.SendPacket(append(pkt.data, []byte{0x00, 0x10}...)) err := s.cryptConn.SendPacket(append(pkt.data, []byte{0x00, 0x10}...))
if err != nil { if err != nil {
s.logger.Warn("Failed to send packet") s.logger.Warn("Failed to send packet", zap.Error(err))
}
} }
time.Sleep(10 * time.Millisecond) time.Sleep(10 * time.Millisecond)
} }