vmangos / core

Progressive Vanilla Core aimed at all versions from 1.2 to 1.12
GNU General Public License v2.0
646 stars 529 forks source link

🐛 [Bug] [CLIENT CRASH] #2658

Closed shudza closed 3 weeks ago

shudza commented 3 weeks ago

🐛 Bug report

Client crashes under certain conditions.

Steps to reproduce

Config:

Compression.Level = 1
Compression.Update.Size = 128
Compression.Movement.Count = 300
---
Visibility.Distance.Continents     = 300
Visibility.Distance.Continents.Min = 100
Visibility.Distance.Instances      = 300
Visibility.Distance.BG             = 300
Visibility.Distance.InFlight       = 300
  1. login
  2. .tele goldshire
  3. hop around wolves
  4. after ~20 secs client crashes

After a while I've noticed that increasing the count limit for compression affects this. Setting Compression.Movement.Count to a high value (thus effectively disabling compression) prevents crash, so I guess that must be the issue. Also my Visibility was abnormaly high so I guess thats why it occured in the first place.

Version & Environment

Client Version:

Operating System:

shudza commented 3 weeks ago

I've debugged some packet sizes in UpdateData.cpp:

Debugger connected to tcp:127.0.0.1:7777
pSize = (size_t) 1842
destsize = (uint32) 1855
pSize = (size_t) 1131
destsize = (uint32) 1144
pSize = (size_t) 32858
destsize = (uint32) 32881
pSize = (size_t) 7930
destsize = (uint32) 7944
pSize = (size_t) 279
destsize = (uint32) 292
pSize = (size_t) 173

destsize = 32881 is too big I think the max packet size is 0x8000 (32676)

EDIT: the destsize log was on the wrong line, after compression it shows lower values: Debugger connected to tcp:127.0.0.1:7777 pSize = (size_t) 1842 destsize = (uint32) 808 pSize = (size_t) 1131 destsize = (uint32) 539 pSize = (size_t) 32889 destsize = (uint32) 10523 pSize = (size_t) 7231 destsize = (uint32) 2291 pSize = (size_t) 203 pSize = (size_t) 195 destsize = (uint32) 56 destsize = (uint32) 42 pSize = (size_t) 206 destsize = (uint32) 163 pSize = (size_t) 438 destsize = (uint32) 364 pSize = (size_t) 429 destsize = (uint32) 347 pSize = (size_t) 350

ratkosrb commented 3 weeks ago

Doesn't seem to crash for me. I set visibility distance to 300 like you said.

shudza commented 3 weeks ago

@ratkosrb can you try setting Continents.MotionUpdate.Threads = 2 as well?

shudza commented 3 weeks ago

Thread safety issue in MovementData::CanAddPacket and MovementData::AddPacket ?

shudza commented 3 weeks ago

This fixes it for me https://github.com/shudza/vmangos/tree/movement-data-lock

ratkosrb commented 3 weeks ago

Thanks for the info. Should be fixed now.