Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Pet: fix heap-use-after-free due to trying to unsummon an already unsummoned pet #478

Closed
wants to merge 1 commit into from
Closed

Pet: fix heap-use-after-free due to trying to unsummon an already unsummoned pet #478

wants to merge 1 commit into from

Conversation

Karth-Xyver
Copy link
Contributor

🍰 Pullrequest

Server kept crashing without crashlog, so I hooked it up to ASAN and got this:

Crashlog 
==2612==ERROR: AddressSanitizer: heap-use-after-free on address 0x1242368d1112 at pc 0x7ff63b3a69e1 bp 0x00c4308ff540 sp 0x00c4308ff548
READ of size 1 at 0x1242368d1112 thread T11
    #0 0x7ff63b3a69e0 in Object::GetTypeId(void) const C:\Server\Source\src\game\Entities\Object.h:415
    #1 0x7ff63c0629ba in Map::RemoveAllObjectsInRemoveList(void) C:\Server\Source\src\game\Maps\Map.cpp:1445
    #2 0x7ff63baa20c2 in MapManager::RemoveAllObjectsInRemoveList(void) C:\Server\Source\src\game\Maps\MapManager.cpp:231
    #3 0x7ff63b26223f in World::Update(unsigned int) C:\Server\Source\src\game\World\World.cpp:1724
    #4 0x7ff63b15a6f9 in WorldRunnable::run(void) C:\Server\Source\src\mangosd\WorldRunnable.cpp:55
    #5 0x7ff63b205818 in MaNGOS::Thread::ThreadTask(void *) C:\Server\Source\src\shared\Multithreading\Threading.cpp:84
    #6 0x7ff63b206416 in std::invoke<void (__cdecl *)(void *), void *>(void (__cdecl *&&)(void *), void *&&) C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.37.32822\include\type_traits:1762
    #7 0x7ff63b205e55 in std::thread::_Invoke<class std::tuple<void (__cdecl *)(void *), void *>, 0, 1>(void *) C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.37.32822\include\thread:55
    #8 0x7ffce04c300f  (C:\Windows\SYSTEM32\ucrtbased.dll+0x1800b300f)
    #9 0x7ffccb8b37ce in __asan::AsanThread::ThreadStart(unsigned __int64) D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_thread.cpp:277
    #10 0x7ffd66ad7033  (C:\Windows\System32\KERNEL32.DLL+0x180017033)
    #11 0x7ffd68482650  (C:\Windows\SYSTEM32\ntdll.dll+0x180052650)

0x1242368d1112 is located 18 bytes inside of 10048-byte region [0x1242368d1100,0x1242368d3840)
freed by thread T11 here:
    #0 0x7ff63e7c19c3 in operator delete(void *, unsigned __int64) D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_win_delete_scalar_size_thunk.cpp:41
    #1 0x7ff63c424b60 in Pet::`scalar deleting dtor'(unsigned int) (C:\Server\Build\bin\x64_Debug\mangosd.exe+0x141404b60)
    #2 0x7ff63c08ac61 in Map::Remove<class Creature>(class Creature *, bool) C:\Server\Source\src\game\Maps\Map.cpp:1072
    #3 0x7ff63c062ad2 in Map::RemoveAllObjectsInRemoveList(void) C:\Server\Source\src\game\Maps\Map.cpp:1464
    #4 0x7ff63baa20c2 in MapManager::RemoveAllObjectsInRemoveList(void) C:\Server\Source\src\game\Maps\MapManager.cpp:231
    #5 0x7ff63b26223f in World::Update(unsigned int) C:\Server\Source\src\game\World\World.cpp:1724
    #6 0x7ff63b15a6f9 in WorldRunnable::run(void) C:\Server\Source\src\mangosd\WorldRunnable.cpp:55
    #7 0x7ff63b205818 in MaNGOS::Thread::ThreadTask(void *) C:\Server\Source\src\shared\Multithreading\Threading.cpp:84
    #8 0x7ff63b206416 in std::invoke<void (__cdecl *)(void *), void *>(void (__cdecl *&&)(void *), void *&&) C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.37.32822\include\type_traits:1762
    #9 0x7ff63b205e55 in std::thread::_Invoke<class std::tuple<void (__cdecl *)(void *), void *>, 0, 1>(void *) C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.37.32822\include\thread:55
    #10 0x7ffce04c300f  (C:\Windows\SYSTEM32\ucrtbased.dll+0x1800b300f)
    #11 0x7ffccb8b37ce in __asan::AsanThread::ThreadStart(unsigned __int64) D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_thread.cpp:277
    #12 0x7ffd66ad7033  (C:\Windows\System32\KERNEL32.DLL+0x180017033)
    #13 0x7ffd68482650  (C:\Windows\SYSTEM32\ntdll.dll+0x180052650)

previously allocated by thread T11 here:
    #0 0x7ff63e7c1935 in operator new(unsigned __int64) D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_win_new_scalar_thunk.cpp:40
    #1 0x7ff63b3e858d in Player::LoadPet(void) C:\Server\Source\src\game\Entities\Player.cpp:16085
    #2 0x7ff63c97d9ba in WorldSession::HandlePlayerLogin(class LoginQueryHolder *) C:\Server\Source\src\game\Entities\CharacterHandler.cpp:932
    #3 0x7ff63c982e85 in PlayerbotHolder::HandlePlayerBotLoginCallback(class QueryResult *, class SqlQueryHolder *) C:\Server\Source\src\game\Entities\CharacterHandler.cpp:136
    #4 0x7ff63c98afca in MaNGOS::_Callback<class PlayerbotHolder, class QueryResult *, class SqlQueryHolder *, void, void>::_Execute(void) C:\Server\Source\src\framework\Utilities\Callback.h:117
    #5 0x7ff63c989349 in MaNGOS::_IQueryCallback<class MaNGOS::_Callback<class PlayerbotHolder, class QueryResult *, class SqlQueryHolder *, void, void>>::Execute(void) C:\Server\Source\src\framework\Utilities\Callback.h:430
    #6 0x7ff63b23e527 in SqlResultQueue::Update(void) C:\Server\Source\src\shared\Database\SqlOperations.cpp:110
    #7 0x7ff63b1bf0c8 in Database::ProcessResultQueue(void) C:\Server\Source\src\shared\Database\Database.cpp:201
    #8 0x7ff63b264a34 in World::UpdateResultQueue(void) C:\Server\Source\src\game\World\World.cpp:2245
    #9 0x7ff63b262126 in World::Update(unsigned int) C:\Server\Source\src\game\World\World.cpp:1695
    #10 0x7ff63b15a6f9 in WorldRunnable::run(void) C:\Server\Source\src\mangosd\WorldRunnable.cpp:55
    #11 0x7ff63b205818 in MaNGOS::Thread::ThreadTask(void *) C:\Server\Source\src\shared\Multithreading\Threading.cpp:84
    #12 0x7ff63b206416 in std::invoke<void (__cdecl *)(void *), void *>(void (__cdecl *&&)(void *), void *&&) C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.37.32822\include\type_traits:1762
    #13 0x7ff63b205e55 in std::thread::_Invoke<class std::tuple<void (__cdecl *)(void *), void *>, 0, 1>(void *) C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.37.32822\include\thread:55
    #14 0x7ffce04c300f  (C:\Windows\SYSTEM32\ucrtbased.dll+0x1800b300f)
    #15 0x7ffccb8b37ce in __asan::AsanThread::ThreadStart(unsigned __int64) D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_thread.cpp:277
    #16 0x7ffd66ad7033  (C:\Windows\System32\KERNEL32.DLL+0x180017033)
    #17 0x7ffd68482650  (C:\Windows\SYSTEM32\ntdll.dll+0x180052650)

Thread T11 created by T0 here:
    #0 0x7ffccb8b59a7 in __asan_wrap_CreateThread D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_win.cpp:163
    #1 0x7ffce04c387e  (C:\Windows\SYSTEM32\ucrtbased.dll+0x1800b387e)
    #2 0x7ff63b206003 in std::thread::_Start<void (__cdecl *)(void *), void *>(void (__cdecl *&&)(void *), void *&&) C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.37.32822\include\thread:73
    #3 0x7ff63b205b74 in std::thread::thread<void (__cdecl *)(void *), void *, 0>(void (__cdecl *&&)(void *), void *&&) C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.37.32822\include\thread:88
    #4 0x7ff63b204a22 in MaNGOS::Thread::Thread(class MaNGOS::Runnable *) C:\Server\Source\src\shared\Multithreading\Threading.cpp:31
    #5 0x7ff63b0d9b8f in Master::Run(void) C:\Server\Source\src\mangosd\Master.cpp:141
    #6 0x7ff63b0c6022 in main C:\Server\Source\src\mangosd\Main.cpp:211
    #7 0x7ff63e7c36f8 in invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78
    #8 0x7ff63e7c364d in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #9 0x7ff63e7c350d in __scrt_common_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:330
    #10 0x7ff63e7c376d in mainCRTStartup D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_main.cpp:16
    #11 0x7ffd66ad7033  (C:\Windows\System32\KERNEL32.DLL+0x180017033)
    #12 0x7ffd68482650  (C:\Windows\SYSTEM32\ntdll.dll+0x180052650)

SUMMARY: AddressSanitizer: heap-use-after-free C:\Server\Source\src\game\Entities\Object.h:415 in Object::GetTypeId(void) const
Shadow bytes around the buggy address:
  0x043e7c19a1d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x043e7c19a1e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x043e7c19a1f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x043e7c19a200: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x043e7c19a210: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x043e7c19a220: fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x043e7c19a230: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x043e7c19a240: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x043e7c19a250: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x043e7c19a260: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x043e7c19a270: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==2612==ABORTING

From the very little I understand from this, Map is trying to unsummon a pet while it's already being unsummoned, causing memory corruption.

No way to reproduce, but it happened after letting celguar's Playerbots run for ~20 minutes.
This could just be due to some quirk of playerbots and not happen during normal usage, but even so I don't think this safeguard would hurt to have.

Proof

  • None

Issues

  • None

How2Test

  • None

Todo / Checklist

  • None

@insunaa
Copy link
Contributor

insunaa commented Oct 31, 2023

good find

@evil-at-wow
Copy link
Contributor

From the very little I understand from this, Map is trying to unsummon a pet while it's already being unsummoned, causing memory corruption.

The map is not trying to unsummon the pet though, it's removing it from the map - presumably because it has been unsummoned indeed. That's basically what Map::RemoveAllObjectsInRemoveList() is doing. When world/grid objects are removed from the map they are put in a 'list of objects to remove' (because they could still be in use) and later actually removed - by Map::RemoveAllObjectsInRemoveList() - from the grid/map and, in C++ terms, deleted. That's why the second stack trace is showing the Pet object to be deleted.

Now, the call to Map::Remove() is a template so it depends on the type of object on how to do this exactly, which is why Map::RemoveAllObjectsInRemoveList() calls Object::GetTypeId() on each of the objects to dispatch this correctly. The fact that this makes ASAN bark (and it says that object was already deleted) means the same Pet object is put in the list multiple times for some reason, and that is the real problem. As you probably noticed, Pet::UnSummon() calls AddObjectToRemoveList() at the end, so the conclusion is indeed that someone is trying to unsummon a pet while it's already being unsummoned, but it's unlikely going to be the Map.

Your change avoids the issue by having Pet::UnSummon() return early, but it's somewhat fighting symptoms because we still don't know what is unsummoning a pet that's already unsummoned, and that would be useful to know. So if you can spare the time, could you run the server under a debugger and put a conditional breakpoint at the beginning of Pet::UnSummon() on the condition that m_removed is true? That way we might catch the offender red-handed...

@Karth-Xyver
Copy link
Contributor Author

Thanks a lot for the clarifications! The same pet is indeed being added to the remove list twice (at least), but I wasn't able to debug it further than that. I'll revert this change and do as you said.

@evil-at-wow
Copy link
Contributor

Thanks! You don't even have to revert the change for that. Even with your change, m_removed will be true before your early return and show us who's trying to unsummon the already unsummoned pet.

@Karth-Xyver
Copy link
Contributor Author

Didn't manage to reproduce it again so far. I'll keep trying and update if it happens.

@killerwife
Copy link
Contributor

I will add an assert to the codebase in this place. I am sure your code is valid but I think we need to treat the cause, not the symptom. Will keep this PR opened, whenver I add an assert, people come running with stack traces.

killerwife added a commit that referenced this pull request Jan 11, 2024
@killerwife
Pet: Add assert for double unsummon of same pet.
7ecbb10 
Diagnostics for this PR:

#478
killerwife added a commit to cmangos/mangos-tbc that referenced this pull request Jan 13, 2024
@killerwife
Pet: Add assert for double unsummon of same pet.
4dde973 
Diagnostics for this PR:

cmangos/mangos-wotlk#478
killerwife added a commit to cmangos/mangos-classic that referenced this pull request Jan 13, 2024
@killerwife
Pet: Add assert for double unsummon of same pet.
5bd9bbb 
Diagnostics for this PR:

cmangos/mangos-wotlk#478
@insunaa
Copy link
Contributor

insunaa commented Jan 17, 2024

Backtrace for the first unsummon call:

#0  Pet::Unsummon (this=0x7fffb4d8f180, mode=PET_SAVE_REAGENTS, owner=0x0)
    at /mnt/980pro/git/wotlk/src/game/Entities/Pet.cpp:772
#1  0x0000555555f940ff in Player::UpdateVisibilityOf (this=0x7fffb1bca4c0, viewPoint=0x7fffb1bca4c0, 
    target=0x7fffb4d8f180) at /mnt/980pro/git/wotlk/src/game/Entities/Player.cpp:21089
#2  0x000055555609bd9a in MaNGOS::VisibleChangesNotifier::Visit (this=0x7fffba1799a8, m=...)
    at /mnt/980pro/git/wotlk/src/game/Grids/GridNotifiers.cpp:36
#3  0x00005555560ef1a6 in VisitorHelper<MaNGOS::VisibleChangesNotifier, Camera> (v=..., c=...)
    at /mnt/980pro/git/wotlk/src/framework/GameSystem/TypeContainerVisitor.h:50
#4  VisitorHelper<MaNGOS::VisibleChangesNotifier, Camera, TypeNull> (v=..., c=...)
    at /mnt/980pro/git/wotlk/src/framework/GameSystem/TypeContainerVisitor.h:57
#5  VisitorHelper<MaNGOS::VisibleChangesNotifier, Corpse, TypeList<Camera, TypeNull> > (v=..., c=...)
    at /mnt/980pro/git/wotlk/src/framework/GameSystem/TypeContainerVisitor.h:58
#6  VisitorHelper<MaNGOS::VisibleChangesNotifier, Creature, TypeList<Corpse, TypeList<Camera, TypeNull> > > (v=..., 
    c=...) at /mnt/980pro/git/wotlk/src/framework/GameSystem/TypeContainerVisitor.h:58
#7  VisitorHelper<MaNGOS::VisibleChangesNotifier, Player, TypeList<Creature, TypeList<Corpse, TypeList<Camera, TypeNull> > > > (v=..., c=...) at /mnt/980pro/git/wotlk/src/framework/GameSystem/TypeContainerVisitor.h:58
#8  VisitorHelper<MaNGOS::VisibleChangesNotifier, TypeList<Player, TypeList<Creature, TypeList<Corpse, TypeList<Camera, TypeNull> > > > > (v=..., c=...) at /mnt/980pro/git/wotlk/src/framework/GameSystem/TypeContainerVisitor.h:65
#9  TypeContainerVisitor<MaNGOS::VisibleChangesNotifier, TypeMapContainer<TypeList<Player, TypeList<Creature, TypeList<Corpse, TypeList<Camera, TypeNull> > > > > >::Visit (this=0x7fffba1799e0, c=...)
    at /mnt/980pro/git/wotlk/src/framework/GameSystem/TypeContainerVisitor.h:80
#10 Grid<Player, TypeList<Player, TypeList<Creature, TypeList<Corpse, TypeList<Camera, TypeNull> > > >, TypeList<GameObject, TypeList<Creature, TypeList<DynamicObject, TypeList<Corpse, TypeNull> > > > >::Visit<MaNGOS::VisibleChangesNotifier>
    (visitor=..., this=<optimized out>) at /mnt/980pro/git/wotlk/src/framework/GameSystem/Grid.h:88
#11 NGrid<8u, Player, TypeList<Player, TypeList<Creature, TypeList<Corpse, TypeList<Camera, TypeNull> > > >, TypeList<GameObject, TypeList<Creature, TypeList<DynamicObject, TypeList<Corpse, TypeNull> > > > >::Visit<MaNGOS::VisibleChangesNotifier, TypeList<Player, TypeList<Creature, TypeList<Corpse, TypeList<Camera, TypeNull> > > > > (visitor=..., 
    this=<optimized out>, x=<optimized out>, y=<optimized out>)
    at /mnt/980pro/git/wotlk/src/framework/GameSystem/NGrid.h:155
#12 Map::Visit<MaNGOS::VisibleChangesNotifier, TypeMapContainer<TypeList<Player, TypeList<Creature, TypeList<Corpse, TypeList<Camera, TypeNull> > > > > > (this=0x55557c63aaf0, cell=..., visitor=...)
    at /mnt/980pro/git/wotlk/src/game/Maps/Map.h:648
#13 Cell::Visit<MaNGOS::VisibleChangesNotifier, TypeMapContainer<TypeList<Player, TypeList<Creature, TypeList<Corpse, TypeList<Camera, TypeNull> > > > > > (this=this@entry=0x7fffba179a20, standing_cell=..., visitor=..., m=..., 
    x=x@entry=-4936.53516, y=y@entry=-1179.00073, radius=<optimized out>)
    at /mnt/980pro/git/wotlk/src/game/Grids/CellImpl.h:102
#14 0x00005555560d8d2f in Cell::Visit<MaNGOS::VisibleChangesNotifier, TypeMapContainer<TypeList<Player, TypeList<Creature, TypeList<Corpse, TypeList<Camera, TypeNull> > > > > > (this=0x7fffba179a20, standing_cell=..., visitor=..., m=..., 
    obj=..., radius=<error reading variable: That operation is not available on integers of more than 8 bytes.>)
    at /mnt/980pro/git/wotlk/src/game/Grids/CellImpl.h:56
#15 Map::UpdateObjectVisibility (this=0x55557c63aaf0, obj=0x7fffb4d8f180, cell=..., cellpair=...)
    at /mnt/980pro/git/wotlk/src/game/Maps/Map.cpp:1298
#16 0x0000555555f1da31 in WorldObject::UpdateObjectVisibility (this=0x65)
    at /mnt/980pro/git/wotlk/src/game/Entities/Object.cpp:2647
#17 0x0000555555fe6fce in Unit::UpdateVisibilityAndView (this=0x7fffb4d8f180)
    at /mnt/980pro/git/wotlk/src/game/Entities/Unit.cpp:9362
#18 0x0000555555ed66f2 in Creature::RemoveCorpse (this=0x7fffb4d8f180, inPlace=<optimized out>)
    at /mnt/980pro/git/wotlk/src/game/Entities/Creature.cpp:309
#19 0x0000555555f30714 in Pet::ForcedDespawn (this=0x7fffb4d8f180, timeMSToDespawn=<optimized out>, 
    onlyAlive=<optimized out>) at /mnt/980pro/git/wotlk/src/game/Entities/Pet.cpp:2444
#20 0x00005555563f8b3a in WorldSession::HandlePetActionHelper (this=this@entry=0x7fff8c00f110, flag=<optimized out>, 
    spellid=spellid@entry=3, petUnit=petUnit@entry=0x7fffb4d8f180, targetGuid=...)
    at /mnt/980pro/git/wotlk/src/game/Entities/PetHandler.cpp:279
#21 0x00005555563f8107 in WorldSession::HandlePetAction (this=0x7fff8c00f110, recv_data=...)
    at /mnt/980pro/git/wotlk/src/game/Entities/PetHandler.cpp:166
#22 0x0000555556233bb7 in WorldSession::ExecuteOpcode (this=this@entry=0x7fff8c00f110, opHandle=..., packet=...)
    at /mnt/980pro/git/wotlk/src/game/Server/WorldSession.cpp:1172
#23 0x000055555623347b in WorldSession::Update (this=0x7fff8c00f110)
    at /mnt/980pro/git/wotlk/src/game/Server/WorldSession.cpp:389
#24 0x00005555562f3e4d in World::UpdateSessions (this=this@entry=0x555557068550, diff=diff@entry=50)
    at /mnt/980pro/git/wotlk/src/game/World/World.cpp:2242
#25 0x0000555556303c13 in World::Update (this=0x555557068550, diff=50)
    at /mnt/980pro/git/wotlk/src/game/World/World.cpp:1662
#26 0x0000555555e106ec in WorldRunnable::run (this=<optimized out>)
    at /mnt/980pro/git/wotlk/src/mangosd/WorldRunnable.cpp:55
#27 0x00007ffff74e1943 in std::execute_native_thread_routine (__p=0x55555abe31f0)
    at /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:104
#28 0x00007ffff71bd9eb in ?? () from /usr/lib/libc.so.6
#29 0x00007ffff72417cc in ?? () from /usr/lib/libc.so.6

backtrace for the second unsummon call:

#0  Pet::Unsummon (this=0x7fffb4d8f180, mode=PET_SAVE_NOT_IN_SLOT, owner=0x7fffb1bca4c0)
    at /mnt/980pro/git/wotlk/src/game/Entities/Pet.cpp:772
#1  0x00005555563f8b3a in WorldSession::HandlePetActionHelper (this=this@entry=0x7fff8c00f110, flag=<optimized out>, 
    spellid=spellid@entry=3, petUnit=petUnit@entry=0x7fffb4d8f180, targetGuid=...)
    at /mnt/980pro/git/wotlk/src/game/Entities/PetHandler.cpp:279
#2  0x00005555563f8107 in WorldSession::HandlePetAction (this=0x7fff8c00f110, recv_data=...)
    at /mnt/980pro/git/wotlk/src/game/Entities/PetHandler.cpp:166
#3  0x0000555556233bb7 in WorldSession::ExecuteOpcode (this=this@entry=0x7fff8c00f110, opHandle=..., packet=...)
    at /mnt/980pro/git/wotlk/src/game/Server/WorldSession.cpp:1172
#4  0x000055555623347b in WorldSession::Update (this=0x7fff8c00f110)
    at /mnt/980pro/git/wotlk/src/game/Server/WorldSession.cpp:389
#5  0x00005555562f3e4d in World::UpdateSessions (this=this@entry=0x555557068550, diff=diff@entry=50)
    at /mnt/980pro/git/wotlk/src/game/World/World.cpp:2242
#6  0x0000555556303c13 in World::Update (this=0x555557068550, diff=50)
    at /mnt/980pro/git/wotlk/src/game/World/World.cpp:1662
#7  0x0000555555e106ec in WorldRunnable::run (this=<optimized out>)
    at /mnt/980pro/git/wotlk/src/mangosd/WorldRunnable.cpp:55
#8  0x00007ffff74e1943 in std::execute_native_thread_routine (__p=0x55555abe31f0)
    at /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:104
#9  0x00007ffff71bd9eb in ?? () from /usr/lib/libc.so.6
#10 0x00007ffff72417cc in ?? () from /usr/lib/libc.so.6

@insunaa
Copy link
Contributor

insunaa commented Jan 17, 2024

The packetlog doesn't show the same opcode being sent twice

ServerToClient: SMSG_COMPRESSED_UPDATE_OBJECT (0x01F6) Length: 29 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.053 Number: 0
Count: 1
[0] UpdateType: Values
[0] GUID: Full: 0x0000000B Type: Player Low: 11
[0] UNIT_FIELD_POWER1: 13884/1.9456E-41

ClientToServer: CMSG_PET_ACTION (0x0175) Length: 20 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.313 Number: 1
GUID: Full: 0xF14000000F000003 Type: Pet Entry: 15 Low: 3
Action: 3
Type: 7 (7)
GUID: 0x0

ServerToClient: SMSG_PET_DISMISS_SOUND (0x0325) Length: 16 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.352 Number: 2
Sound ID: 371
Position: X: -4936.535 Y: -1179.0007 Z: 501.69452

ServerToClient: SMSG_AURA_UPDATE (0x0496) Length: 10 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.352 Number: 3
GUID: Full: 0xF14000000F000003 Type: Pet Entry: 15 Low: 3
[0] Slot: 5
[0] Spell ID: 0 (0)

ServerToClient: SMSG_COOLDOWN_EVENT (0x0135) Length: 12 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.352 Number: 4
Spell ID: 4511 (4511)
GUID: Full: 0xF14000000F000003 Type: Pet Entry: 15 Low: 3

ServerToClient: SMSG_AURA_UPDATE (0x0496) Length: 10 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.352 Number: 5
GUID: Full: 0xF14000000F000003 Type: Pet Entry: 15 Low: 3
[0] Slot: 4
[0] Spell ID: 0 (0)

ServerToClient: SMSG_PET_SPELLS_MESSAGE (0x0179) Length: 8 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.352 Number: 6
GUID: 0x0

ServerToClient: SMSG_POWER_UPDATE (0x0480) Length: 10 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.352 Number: 7
GUID: Full: 0xF14000000F000003 Type: Pet Entry: 15 Low: 3
Power type: 0 (Mana)
Value: 7268

ServerToClient: SMSG_POWER_UPDATE (0x0480) Length: 10 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.352 Number: 8
GUID: Full: 0xF14000000F000003 Type: Pet Entry: 15 Low: 3
Power type: 0 (Mana)
Value: 3228

ServerToClient: SMSG_AURA_UPDATE (0x0496) Length: 10 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.352 Number: 9
GUID: Full: 0xF14000000F000003 Type: Pet Entry: 15 Low: 3
[0] Slot: 0
[0] Spell ID: 0 (0)

ServerToClient: SMSG_AURA_UPDATE (0x0496) Length: 10 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.352 Number: 10
GUID: Full: 0xF14000000F000003 Type: Pet Entry: 15 Low: 3
[0] Slot: 1
[0] Spell ID: 0 (0)

ServerToClient: SMSG_AURA_UPDATE (0x0496) Length: 10 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.352 Number: 11
GUID: Full: 0xF14000000F000003 Type: Pet Entry: 15 Low: 3
[0] Slot: 2
[0] Spell ID: 0 (0)

ServerToClient: SMSG_AURA_UPDATE (0x0496) Length: 10 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.352 Number: 12
GUID: Full: 0xF14000000F000003 Type: Pet Entry: 15 Low: 3
[0] Slot: 3
[0] Spell ID: 0 (0)

ServerToClient: SMSG_DESTROY_OBJECT (0x00AA) Length: 9 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.352 Number: 13
GUID: Full: 0xF14000000F000003 Type: Pet Entry: 15 Low: 3
Despawn Animation: true

ServerToClient: SMSG_PET_SPELLS_MESSAGE (0x0179) Length: 8 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.352 Number: 14
GUID: 0x0

ServerToClient: SMSG_AURA_UPDATE (0x0496) Length: 7 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.352 Number: 15
GUID: Full: 0x0000000B Type: Player Low: 11
[0] Slot: 2
[0] Spell ID: 0 (0)

ServerToClient: SMSG_COMPRESSED_UPDATE_OBJECT (0x01F6) Length: 37 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.354 Number: 16
Count: 1
[0] UpdateType: Values
[0] GUID: Full: 0x0000000B Type: Player Low: 11
[0] UNIT_FIELD_SUMMON: 0/0
[0] UNIT_FIELD_SUMMON + 1: 0/0
[0] UNIT_FIELD_HEALTH: 16566
[0] UNIT_FIELD_POWER1: 13911/1.9493E-41
[0] UNIT_FIELD_MAXHEALTH: 16566

ClientToServer: CMSG_QUERY_PET_NAME (0x0052) Length: 12 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.407 Number: 17
Pet number: 15
Guid: Full: 0xF14000000F000003 Type: Pet Entry: 15 Low: 3

ServerToClient: SMSG_QUERY_PET_NAME_RESPONSE (0x0053) Length: 10 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.452 Number: 18
Pet number: 15
Pet name:

ServerToClient: SMSG_ON_MONSTER_MOVE (0x00DD) Length: 60 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.853 Number: 19
GUID: Full: 0xF1300015DB000039 Type: Creature Entry: 5595 Low: 57
Toggle AnimTierInTrans: false
Position: X: -4965.72 Y: -1242.6991 Z: 502.00082
Move Ticks: 5653
Spline Type: 0 (Normal)
Spline Flags: 4096 (WalkMode)
Move Time: 10624
Waypoints: 4
Waypoint Endpoint: X: -4941.09 Y: -1251.62 Z: 501.648
[1] Waypoint: X: -4963.1553 Y: -1245.1595 Z: 501.8244
[2] Waypoint: X: -4955.4053 Y: -1247.1595 Z: 502.0744
[3] Waypoint: X: -4948.1553 Y: -1249.4095 Z: 502.0744

ServerToClient: SMSG_EMOTE (0x0103) Length: 12 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:10.954 Number: 20
Emote ID: 6 (OneShotQuestion)
GUID: Full: 0xF130009AC70010B6 Type: Creature Entry: 39623 Low: 4278

ClientToServer: CMSG_MESSAGECHAT (0x0095) Length: 27 ConnIdx: 0 EP: [::ffff:192.168.0.11]:42978 Time: 01/17/2024 07:51:11.047 Number: 21
Type: 1 (Say)
Language: 7 (Common)
Message: .debug packetlog 0

so it's not some issue of repeat packets at least

@insunaa
Copy link
Contributor

insunaa commented Jan 17, 2024 •
edited
Loading

ahh, duh. the problem is RemoveCorpse(true); // force corpse removal in the same grid

Either RemoveCorpse(true); must go or Unsummon(PET_SAVE_NOT_IN_SLOT, owner); must go. Both of them eventually do the same thing. RemoveCorpse() calls Unsummon() and Unsummon() queues the corpse for removal through AddObjectToRemoveList()

My vote is on removing RemoveCorpse(true);
because that one eventually calls static_cast<Pet*>(creature)->Unsummon(PET_SAVE_REAGENTS);
and we don't want to save reagents

@insunaa
Copy link
Contributor

insunaa commented Jan 28, 2024 

diff --git a/src/game/Entities/Pet.cpp b/src/game/Entities/Pet.cpp
index cf8f8e509..2be503a30 100644
--- a/src/game/Entities/Pet.cpp
+++ b/src/game/Entities/Pet.cpp
@@ -2441,8 +2441,6 @@ void Pet::ForcedDespawn(uint32 timeMSToDespawn, bool onlyAlive)
     if (IsAlive())
         SetDeathState(JUST_DIED);
 
-    RemoveCorpse(true);                                     // force corpse removal in the same grid
-
     Unsummon(PET_SAVE_NOT_IN_SLOT, owner);
 }

change the PR to this and it will work

@insunaa insunaa mentioned this pull request Mar 14, 2024
Closed
1 task
@insunaa insunaa closed this Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Karth-Xyver @insunaa @evil-at-wow @killerwife