The-Cataclysm-Preservation-Project / TrinityCore

Archived repository for WoW 4.3.4.15595. The project will be reworked for Cataclysm Classic as an official new branch of TrinityCore at https://github.com/TrinityCore/TrinityCore
GNU General Public License v2.0
237 stars 97 forks source link

BWD Onyxia elevator crash #310

Closed roc13x closed 2 years ago

roc13x commented 2 years ago

Description:

The Onyxia elevator in BWD causes a core crash

Current behaviour:

During the Nefarian fight in BWD, the central elevator is lowered to submerge the room in lava. When the elevator is fully lowered, the core crashes. I did a bit of investigation and it seems that in the TransportAnimation::GetAnimNode iterator, it attempts to get the previous element, while already on the first element. Triggered when a transport reaches its final stop frame. Stacktrace: https://gist.github.com/roc13x/241c54e394033c16e59254930c7b8768

Expected behaviour:

It should not crash

Steps to reproduce the problem:

  1. Start the Nefarian encounter in Blackwing Descent
  2. When Onyxia is killed, the platform lowers into the lava
  3. Wait for it to fully lower, and the core will crash immediately

TC rev. hash/commit: d5db24c3d8848487387894107ff76bcbe8728641

TrinityCore rev. d5db24c3d884+ 2022-01-01 22:11:26 +0000 (master branch) (Win64, Debug, Static) (worldserver-daemon)

TDB version: 434.21071

Operating system: Windows 10

Ovahlord commented 2 years ago

I've heard of that one before but I cannot reproduce it at all. Is there any special setting that you are using?

roc13x commented 2 years ago

I've done some tests, and found that it only happens when the core is compiled in Debug mode. Release or RelWithDebInfo doesn't crash.

In fact, my debug prints show something interesting when in Release mode:

GetAnimNode iterator start - time: 21663
element 21867
element 21767
element 21700
element 21667
element 16867 - is <= time, getting previous element
got previous element: 21667
percPos = 0.999167

GetAnimNode iterator start - time: 21789
element 21867
element 21767 - is <= time, getting previous element
got previous element: 21867
percPos = 0.220000

GetAnimNode iterator start - time: 21867
element 21867 - is <= time, getting previous element
got previous element: 21767
percPos = 0.000000

See how on the very last tick, it actually fetched the next element rather than the previous? Even though the code always does --itr. Some kind of compiler optimisation thats not done in debug mode?

Ovahlord commented 2 years ago

My blind guess would be that debug mode takes the returns more serious so the return within the loop is just without effect because it's called within the loop and not outside of it, causing the loop to just continue, eventually hitting a dead end.

roc13x commented 2 years ago

Hm, must be something like that. Anyway, since its an edge issue only affecting full Debug mode, I'll close this one