Trinitycore: [Crash] - FlightPathMovementGenerator::DoEventIfAny

[Jildor]

Description:
Crash in: https://github.com/TrinityCore/TrinityCore/blob/3.3.5/src/server/game/Movement/MovementGenerators/WaypointMovementGenerator.cpp#L481

Crashlog: https://gist.github.com/Jildor/269ea12e2225c67732d448d34eabb747
Old crashlog (rev about December 2015): https://gist.github.com/Jildor/7302fc18ff533bbe3cb0655d1f75ed10

Branch(es): 3.3.5

TC rev. hash/commit: 445c5a00e8

TDB version: TDB335.63

Operating system: Debian

[ghost]

oh!

[ghost]

Does this happen when you change the config to use instant Flight Paths? (And conversely, does it not happen if you use the default setting?)

[Jildor]

@tkrokli I have:

AllFlightPaths = 0
InstantFlightPaths = 0

[Jildor]

I add a new crashlog with info registers and x/32i $pc for if it helps in anything
https://gist.github.com/Jildor/6481a96304b7f5ea64248c2b4aad95d0

[Aokromes]

more than 1 year since the last message, if this bug is still valid open a ticket with updated core.

[jackpoz]

Note to self:

• [ ] add GetDebugInfo() to FlightPathMovementGenerator
• [ ] add GetDebugInfo() to the whole class hierarchy
• [ ] assert on node in FlightPathMovementGenerator::DoEventIfAny()

[TheWinchesters]

Rev. a040d484437b2f12ce88dc4372bfacc18204c8fc
Core has custom stuff (Transmog, anticheat, crossfaction bg, stuff not related to this problem)
Using Debug mode
https://gist.github.com/TheWinchesters/663241edc606ad606b7b46aeecd33316

Rev. 29ca0c56d9551bcdf4e111ffb7538c1c1cd5f896
Core has custom stuff (Transmog, anticheat, crossfaction bg, stuff not related to this problem)
Using RelWithDebInfo mode
https://gist.github.com/TheWinchesters/2ad2e83b8ea4dc0e2a02caf967153310

Additional info:
The crash is not often (once in a week, or once in a month) and it does not happen again after worldserver restarts.

[jackpoz]

In https://github.com/TrinityCore/TrinityCore/blob/3.3.5/src/server/game/Movement/MovementGenerators/FlightPathMovementGenerator.cpp#L90 we check only if the current node is not more than movespline->currentPathIdx() , but there is no relation about the spline being the actual FlightPath spline, it could be whatever spline.

Maybe an assert there could help too.

[Shauren]

Maybe we should start tracking the spline id in movement generators (its already set properly on spline)

https://github.com/TrinityCore/TrinityCore/blob/7b8f294c024230451f82ff67150e11fc31b61293/src/server/game/Movement/Spline/MoveSplineInit.cpp#L186-L188

[jackpoz]

https://github.com/TrinityCore/TrinityCore/commit/9da81239596bf798b1beafc5ddeaeb37dcc330ff is supposed to trigger an assert if node is null (in case that's the issue), please update and post the new crashlog

[TheWinchesters]

New crashlog https://gist.github.com/TheWinchesters/6aaa3791f4d6323c05e0a8c3ddee2213
Rev: https://github.com/TrinityCore/TrinityCore/commit/f9a366628a17263852634a1095edcdb79a8b383e

[jackpoz]

Relevant info:

Current Node: 44
Mode: 0 Priority: 2 Flags: 2 BaseUniteState: 256
Start Path Id: 25 Path Size: 44 HasArrived: true End Grid X: -0.567491 End Grid Y: -861.307 End Map Id: 0 Preloaded Target Node: 41

[jackpoz]

https://github.com/TrinityCore/TrinityCore/commit/560b7e33974904938f3f85a2f78c0850df2f9333 , https://github.com/TrinityCore/TrinityCore/commit/9ed35e2108d9b33ad579a7247ce51f78d4ff0c5b and https://github.com/TrinityCore/TrinityCore/commit/a9f1dc1da72e217874a5191d7740ebec36803b3e add some more info to the crash log

[jackpoz]

From the crashlog looks like an out of bounds access, I added an assert to stop as soon as that happens, maybe there is a particular flypath when that happens.
As @Shauren said we should link these movement generators to the spline they start and handle gracefully the case where they get desync'd with just a TC_LOG_WARN

[jackpoz]

maybe a player on flypath gets an arena/bg, goes in, charges/gets death grip, gets out, crashes. And maybe it needs to be on the last bit of the flypath, so when it's resumed there's nothing left to do.

Something similar to https://github.com/TrinityCore/TrinityCore/issues/23077 could be implemented. I would say we should check also the return value of init.Launch() in case validation fails and it returns 0.

[jackpoz]

This has been fixed