From c04fa504cc9b19ace65e60ddd148962182cd076c Mon Sep 17 00:00:00 2001 From: Houmgaor Date: Sat, 21 Feb 2026 13:39:44 +0100 Subject: [PATCH] refactor(channelserver): extract UserBinaryStore and MinidataStore The userBinary and minidata maps with their locks were spread across Server as raw fields with manual lock management. Cross-channel session searches also required acquiring nested locks (server lock + binary lock). Encapsulating in dedicated types eliminates the nested locking and reduces Server's field count by 4. --- .../channelserver/channel_registry_local.go | 8 +-- server/channelserver/channel_registry_test.go | 2 +- server/channelserver/handlers_data.go | 4 +- server/channelserver/handlers_data_test.go | 2 +- server/channelserver/handlers_misc.go | 9 +-- server/channelserver/handlers_object.go | 4 +- server/channelserver/handlers_plate.go | 18 ++---- .../handlers_savedata_integration_test.go | 2 +- server/channelserver/handlers_session.go | 7 +-- server/channelserver/handlers_users.go | 8 +-- server/channelserver/handlers_users_test.go | 7 +-- server/channelserver/minidata_store.go | 29 ++++++++++ .../session_lifecycle_integration_test.go | 4 +- server/channelserver/sys_channel_server.go | 19 ++----- server/channelserver/user_binary_store.go | 56 +++++++++++++++++++ 15 files changed, 111 insertions(+), 68 deletions(-) create mode 100644 server/channelserver/minidata_store.go create mode 100644 server/channelserver/user_binary_store.go diff --git a/server/channelserver/channel_registry_local.go b/server/channelserver/channel_registry_local.go index a04651d0e..f0239ac6e 100644 --- a/server/channelserver/channel_registry_local.go +++ b/server/channelserver/channel_registry_local.go @@ -76,7 +76,6 @@ func (r *LocalChannelRegistry) SearchSessions(predicate func(SessionSnapshot) bo break } c.Lock() - c.userBinaryPartsLock.RLock() for _, session := range c.sessions { if len(results) >= max { break @@ -90,16 +89,11 @@ func (r *LocalChannelRegistry) SearchSessions(predicate func(SessionSnapshot) bo if session.stage != nil { snap.StageID = session.stage.id } - ub3 := c.userBinaryParts[userBinaryPartID{charID: session.charID, index: 3}] - if len(ub3) > 0 { - snap.UserBinary3 = make([]byte, len(ub3)) - copy(snap.UserBinary3, ub3) - } + snap.UserBinary3 = c.userBinary.GetCopy(session.charID, 3) if predicate(snap) { results = append(results, snap) } } - c.userBinaryPartsLock.RUnlock() c.Unlock() } return results diff --git a/server/channelserver/channel_registry_test.go b/server/channelserver/channel_registry_test.go index bdeccffbf..2fe6b296c 100644 --- a/server/channelserver/channel_registry_test.go +++ b/server/channelserver/channel_registry_test.go @@ -14,7 +14,7 @@ func createTestChannels(count int) []*Server { s.IP = "10.0.0.1" s.Port = uint16(54001 + i) s.GlobalID = "0101" - s.userBinaryParts = make(map[userBinaryPartID][]byte) + s.userBinary = NewUserBinaryStore() channels[i] = s } return channels diff --git a/server/channelserver/handlers_data.go b/server/channelserver/handlers_data.go index bbf9df754..84805fc8a 100644 --- a/server/channelserver/handlers_data.go +++ b/server/channelserver/handlers_data.go @@ -193,9 +193,7 @@ func handleMsgMhfLoaddata(s *Session, p mhfpacket.MHFPacket) { bf := byteframe.NewByteFrameFromBytes(decompSaveData) _, _ = bf.Seek(88, io.SeekStart) name := bf.ReadNullTerminatedBytes() - s.server.userBinaryPartsLock.Lock() - s.server.userBinaryParts[userBinaryPartID{charID: s.charID, index: 1}] = append(name, []byte{0x00}...) - s.server.userBinaryPartsLock.Unlock() + s.server.userBinary.Set(s.charID, 1, append(name, []byte{0x00}...)) s.Name, _ = stringsupport.SJISToUTF8(name) } diff --git a/server/channelserver/handlers_data_test.go b/server/channelserver/handlers_data_test.go index 5286d2ca3..0078463af 100644 --- a/server/channelserver/handlers_data_test.go +++ b/server/channelserver/handlers_data_test.go @@ -443,7 +443,7 @@ func TestHandleMsgMhfLoaddata_Integration(t *testing.T) { s := createTestSession(mock) s.charID = charID SetTestDB(s.server, db) - s.server.userBinaryParts = make(map[userBinaryPartID][]byte) + s.server.userBinary = NewUserBinaryStore() pkt := &mhfpacket.MsgMhfLoaddata{ AckHandle: 5678, diff --git a/server/channelserver/handlers_misc.go b/server/channelserver/handlers_misc.go index 8ddf733a8..8a0dafbe0 100644 --- a/server/channelserver/handlers_misc.go +++ b/server/channelserver/handlers_misc.go @@ -217,10 +217,7 @@ func handleMsgMhfUseUdShopCoin(s *Session, p mhfpacket.MHFPacket) {} func handleMsgMhfGetEnhancedMinidata(s *Session, p mhfpacket.MHFPacket) { pkt := p.(*mhfpacket.MsgMhfGetEnhancedMinidata) - s.server.minidataLock.RLock() - data, ok := s.server.minidataParts[pkt.CharID] - s.server.minidataLock.RUnlock() - + data, ok := s.server.minidata.Get(pkt.CharID) if !ok { data = make([]byte, 1) } @@ -231,9 +228,7 @@ func handleMsgMhfSetEnhancedMinidata(s *Session, p mhfpacket.MHFPacket) { pkt := p.(*mhfpacket.MsgMhfSetEnhancedMinidata) dumpSaveData(s, pkt.RawDataPayload, "minidata") - s.server.minidataLock.Lock() - s.server.minidataParts[s.charID] = pkt.RawDataPayload - s.server.minidataLock.Unlock() + s.server.minidata.Set(s.charID, pkt.RawDataPayload) doAckSimpleSucceed(s, pkt.AckHandle, []byte{0x00, 0x00, 0x00, 0x00}) } diff --git a/server/channelserver/handlers_object.go b/server/channelserver/handlers_object.go index eb9cb1151..ee991161b 100644 --- a/server/channelserver/handlers_object.go +++ b/server/channelserver/handlers_object.go @@ -72,9 +72,7 @@ func handleMsgSysSetObjectBinary(s *Session, p mhfpacket.MHFPacket) { /* This causes issues with PS3 as this actually sends with endiness! for _, session := range s.server.sessions { if session.charID == s.charID { - s.server.userBinaryPartsLock.Lock() - s.server.userBinaryParts[userBinaryPartID{charID: s.charID, index: 3}] = pkt.RawDataPayload - s.server.userBinaryPartsLock.Unlock() + s.server.userBinary.Set(s.charID, 3, pkt.RawDataPayload) msg := &mhfpacket.MsgSysNotifyUserBinary{ CharID: s.charID, BinaryType: 3, diff --git a/server/channelserver/handlers_plate.go b/server/channelserver/handlers_plate.go index 1f417c9f3..d1462ddb7 100644 --- a/server/channelserver/handlers_plate.go +++ b/server/channelserver/handlers_plate.go @@ -131,10 +131,8 @@ func handleMsgMhfSavePlateData(s *Session, p mhfpacket.MHFPacket) { // Invalidate user binary cache so other players see updated appearance // User binary types 2 and 3 contain equipment/appearance data - s.server.userBinaryPartsLock.Lock() - delete(s.server.userBinaryParts, userBinaryPartID{charID: s.charID, index: 2}) - delete(s.server.userBinaryParts, userBinaryPartID{charID: s.charID, index: 3}) - s.server.userBinaryPartsLock.Unlock() + s.server.userBinary.Delete(s.charID, 2) + s.server.userBinary.Delete(s.charID, 3) saveDuration := time.Since(saveStart) s.logger.Info("PlateData saved successfully", @@ -213,10 +211,8 @@ func handleMsgMhfSavePlateBox(s *Session, p mhfpacket.MHFPacket) { } // Invalidate user binary cache so other players see updated appearance - s.server.userBinaryPartsLock.Lock() - delete(s.server.userBinaryParts, userBinaryPartID{charID: s.charID, index: 2}) - delete(s.server.userBinaryParts, userBinaryPartID{charID: s.charID, index: 3}) - s.server.userBinaryPartsLock.Unlock() + s.server.userBinary.Delete(s.charID, 2) + s.server.userBinary.Delete(s.charID, 3) doAckSimpleSucceed(s, pkt.AckHandle, []byte{0x00, 0x00, 0x00, 0x00}) } @@ -258,10 +254,8 @@ func handleMsgMhfSavePlateMyset(s *Session, p mhfpacket.MHFPacket) { } // Invalidate user binary cache so other players see updated appearance - s.server.userBinaryPartsLock.Lock() - delete(s.server.userBinaryParts, userBinaryPartID{charID: s.charID, index: 2}) - delete(s.server.userBinaryParts, userBinaryPartID{charID: s.charID, index: 3}) - s.server.userBinaryPartsLock.Unlock() + s.server.userBinary.Delete(s.charID, 2) + s.server.userBinary.Delete(s.charID, 3) doAckSimpleSucceed(s, pkt.AckHandle, []byte{0x00, 0x00, 0x00, 0x00}) } diff --git a/server/channelserver/handlers_savedata_integration_test.go b/server/channelserver/handlers_savedata_integration_test.go index f02e3507b..f84bca92b 100644 --- a/server/channelserver/handlers_savedata_integration_test.go +++ b/server/channelserver/handlers_savedata_integration_test.go @@ -507,7 +507,7 @@ func TestSaveLoad_CompleteSaveLoadCycle(t *testing.T) { s2 := createTestSession(mock2) s2.charID = charID SetTestDB(s2.server, db) - s2.server.userBinaryParts = make(map[userBinaryPartID][]byte) + s2.server.userBinary = NewUserBinaryStore() // Load character data loadPkt := &mhfpacket.MsgMhfLoaddata{ diff --git a/server/channelserver/handlers_session.go b/server/channelserver/handlers_session.go index ffbaf47a9..b31d532da 100644 --- a/server/channelserver/handlers_session.go +++ b/server/channelserver/handlers_session.go @@ -536,7 +536,6 @@ func handleMsgMhfTransitMessage(s *Session, p mhfpacket.MHFPacket) { break } c.Lock() - c.userBinaryPartsLock.RLock() for _, session := range c.sessions { if count == maxResults { break @@ -551,19 +550,15 @@ func handleMsgMhfTransitMessage(s *Session, p mhfpacket.MHFPacket) { continue } count++ - ub3 := c.userBinaryParts[userBinaryPartID{charID: session.charID, index: 3}] - ub3Copy := make([]byte, len(ub3)) - copy(ub3Copy, ub3) results = append(results, sessionResult{ charID: session.charID, name: stringsupport.UTF8ToSJIS(session.Name), stageID: stringsupport.UTF8ToSJIS(session.stage.id), ip: net.ParseIP(c.IP).To4(), port: c.Port, - userBin3: ub3Copy, + userBin3: c.userBinary.GetCopy(session.charID, 3), }) } - c.userBinaryPartsLock.RUnlock() c.Unlock() } diff --git a/server/channelserver/handlers_users.go b/server/channelserver/handlers_users.go index dff556472..deb571871 100644 --- a/server/channelserver/handlers_users.go +++ b/server/channelserver/handlers_users.go @@ -15,9 +15,7 @@ func handleMsgSysSetUserBinary(s *Session, p mhfpacket.MHFPacket) { s.logger.Warn("Invalid BinaryType", zap.Uint8("type", pkt.BinaryType)) return } - s.server.userBinaryPartsLock.Lock() - s.server.userBinaryParts[userBinaryPartID{charID: s.charID, index: pkt.BinaryType}] = pkt.RawDataPayload - s.server.userBinaryPartsLock.Unlock() + s.server.userBinary.Set(s.charID, pkt.BinaryType, pkt.RawDataPayload) s.server.BroadcastMHF(&mhfpacket.MsgSysNotifyUserBinary{ CharID: s.charID, @@ -28,9 +26,7 @@ func handleMsgSysSetUserBinary(s *Session, p mhfpacket.MHFPacket) { func handleMsgSysGetUserBinary(s *Session, p mhfpacket.MHFPacket) { pkt := p.(*mhfpacket.MsgSysGetUserBinary) - s.server.userBinaryPartsLock.RLock() - data, ok := s.server.userBinaryParts[userBinaryPartID{charID: pkt.CharID, index: pkt.BinaryType}] - s.server.userBinaryPartsLock.RUnlock() + data, ok := s.server.userBinary.Get(pkt.CharID, pkt.BinaryType) if !ok { doAckBufFail(s, pkt.AckHandle, make([]byte, 4)) diff --git a/server/channelserver/handlers_users_test.go b/server/channelserver/handlers_users_test.go index fffe786d9..455cffec7 100644 --- a/server/channelserver/handlers_users_test.go +++ b/server/channelserver/handlers_users_test.go @@ -50,12 +50,11 @@ func TestHandleMsgSysNotifyUserBinary(t *testing.T) { func TestHandleMsgSysGetUserBinary_FromCache(t *testing.T) { server := createMockServer() - server.userBinaryParts = make(map[userBinaryPartID][]byte) + server.userBinary = NewUserBinaryStore() session := createMockSession(1, server) // Pre-populate cache - key := userBinaryPartID{charID: 100, index: 1} - server.userBinaryParts[key] = []byte{0x01, 0x02, 0x03, 0x04} + server.userBinary.Set(100, 1, []byte{0x01, 0x02, 0x03, 0x04}) pkt := &mhfpacket.MsgSysGetUserBinary{ AckHandle: 12345, @@ -78,7 +77,7 @@ func TestHandleMsgSysGetUserBinary_FromCache(t *testing.T) { func TestHandleMsgSysGetUserBinary_NotInCache(t *testing.T) { server := createMockServer() - server.userBinaryParts = make(map[userBinaryPartID][]byte) + server.userBinary = NewUserBinaryStore() session := createMockSession(1, server) pkt := &mhfpacket.MsgSysGetUserBinary{ diff --git a/server/channelserver/minidata_store.go b/server/channelserver/minidata_store.go new file mode 100644 index 000000000..6cce64178 --- /dev/null +++ b/server/channelserver/minidata_store.go @@ -0,0 +1,29 @@ +package channelserver + +import "sync" + +// MinidataStore is a thread-safe store for per-character enhanced minidata. +type MinidataStore struct { + mu sync.RWMutex + data map[uint32][]byte +} + +// NewMinidataStore creates an empty MinidataStore. +func NewMinidataStore() *MinidataStore { + return &MinidataStore{data: make(map[uint32][]byte)} +} + +// Get returns the minidata for the given character ID. +func (s *MinidataStore) Get(charID uint32) ([]byte, bool) { + s.mu.RLock() + data, ok := s.data[charID] + s.mu.RUnlock() + return data, ok +} + +// Set stores minidata for the given character ID. +func (s *MinidataStore) Set(charID uint32, data []byte) { + s.mu.Lock() + s.data[charID] = data + s.mu.Unlock() +} diff --git a/server/channelserver/session_lifecycle_integration_test.go b/server/channelserver/session_lifecycle_integration_test.go index 497da6a21..6e082d4d9 100644 --- a/server/channelserver/session_lifecycle_integration_test.go +++ b/server/channelserver/session_lifecycle_integration_test.go @@ -583,8 +583,8 @@ func createTestServerWithDB(t *testing.T, db *sqlx.DB) *Server { db: db, sessions: make(map[net.Conn]*Session), stages: make(map[string]*Stage), - userBinaryParts: make(map[userBinaryPartID][]byte), - minidataParts: make(map[uint32][]byte), + userBinary: NewUserBinaryStore(), + minidata: NewMinidataStore(), semaphore: make(map[string]*Semaphore), erupeConfig: &cfg.Config{ RealClientMode: cfg.ZZ, diff --git a/server/channelserver/sys_channel_server.go b/server/channelserver/sys_channel_server.go index c15182d4e..ad092d29b 100644 --- a/server/channelserver/sys_channel_server.go +++ b/server/channelserver/sys_channel_server.go @@ -29,12 +29,6 @@ type Config struct { Enable bool } -// Map key type for a user binary part. -type userBinaryPartID struct { - charID uint32 - index uint8 -} - // Server is a MHF channel server. type Server struct { sync.Mutex @@ -81,13 +75,8 @@ type Server struct { // Used to map different languages i18n i18n - // UserBinary - userBinaryPartsLock sync.RWMutex - userBinaryParts map[userBinaryPartID][]byte - - // EnhancedMinidata - minidataLock sync.RWMutex - minidataParts map[uint32][]byte + userBinary *UserBinaryStore + minidata *MinidataStore // Semaphore semaphoreLock sync.RWMutex @@ -118,8 +107,8 @@ func NewServer(config *Config) *Server { done: make(chan struct{}), sessions: make(map[net.Conn]*Session), stages: make(map[string]*Stage), - userBinaryParts: make(map[userBinaryPartID][]byte), - minidataParts: make(map[uint32][]byte), + userBinary: NewUserBinaryStore(), + minidata: NewMinidataStore(), semaphore: make(map[string]*Semaphore), semaphoreIndex: 7, discordBot: config.DiscordBot, diff --git a/server/channelserver/user_binary_store.go b/server/channelserver/user_binary_store.go new file mode 100644 index 000000000..9db6ab263 --- /dev/null +++ b/server/channelserver/user_binary_store.go @@ -0,0 +1,56 @@ +package channelserver + +import "sync" + +// userBinaryPartID is the composite key for a user binary part. +type userBinaryPartID struct { + charID uint32 + index uint8 +} + +// UserBinaryStore is a thread-safe store for per-character binary data parts. +type UserBinaryStore struct { + mu sync.RWMutex + data map[userBinaryPartID][]byte +} + +// NewUserBinaryStore creates an empty UserBinaryStore. +func NewUserBinaryStore() *UserBinaryStore { + return &UserBinaryStore{data: make(map[userBinaryPartID][]byte)} +} + +// Get returns the binary data for the given character and index. +func (s *UserBinaryStore) Get(charID uint32, index uint8) ([]byte, bool) { + s.mu.RLock() + data, ok := s.data[userBinaryPartID{charID: charID, index: index}] + s.mu.RUnlock() + return data, ok +} + +// GetCopy returns a copy of the binary data, safe for use after the lock is released. +func (s *UserBinaryStore) GetCopy(charID uint32, index uint8) []byte { + s.mu.RLock() + src := s.data[userBinaryPartID{charID: charID, index: index}] + if len(src) == 0 { + s.mu.RUnlock() + return nil + } + dst := make([]byte, len(src)) + copy(dst, src) + s.mu.RUnlock() + return dst +} + +// Set stores binary data for the given character and index. +func (s *UserBinaryStore) Set(charID uint32, index uint8, data []byte) { + s.mu.Lock() + s.data[userBinaryPartID{charID: charID, index: index}] = data + s.mu.Unlock() +} + +// Delete removes binary data for the given character and index. +func (s *UserBinaryStore) Delete(charID uint32, index uint8) { + s.mu.Lock() + delete(s.data, userBinaryPartID{charID: charID, index: index}) + s.mu.Unlock() +}