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

CHANGELOG.md

@@ -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
 - 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
 

schemas/patch-schema/27-fix-character-defaults.sql

@@ -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;

server/channelserver/handlers_stage.go

@@ -97,7 +97,8 @@ func doStageTransfer(s *Session, ackHandle uint32, stageID string) {
 	s.stage = s.server.stages[stageID]
 	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{})
 
 	// Confirm the stage entry
@@ -151,10 +152,12 @@ func doStageTransfer(s *Session, ackHandle uint32, stageID string) {
 		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.
-	if len(newNotif.Data()) > 2 {
+	s.QueueSend(newNotif.Data())
-		s.QueueSend(newNotif.Data())
-	}
 }
 
 func destructEmptyStages(s *Session) {
@@ -172,17 +175,36 @@ func destructEmptyStages(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.
 	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")
+	var objectsToDelete []*Object
 	for _, object := range s.stage.objects {
 		if object.ownerCharID == s.charID {
-			s.stage.BroadcastMHF(&mhfpacket.MsgSysDeleteObject{ObjID: object.id}, s)
+			objectsToDelete = append(objectsToDelete, object)
-			delete(s.stage.objects, object.ownerCharID)
 		}
 	}
+
+	// Delete from map while still holding lock
+	for _, object := range objectsToDelete {
+		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)
 	destructEmptySemaphores(s)
 }

server/channelserver/sys_session.go

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