Trinitycore: [CRASH] Duplicate threat list entry being inserted - memory leak

[n4ndo]

Description:
Trinity::Assert (file=file@entry=0x23e1240 "/Sources/src/server/game/Combat/ThreatManager.cpp", line=line@entry=687, function=function@entry=0x23e15f0 "PutThreatListRef", message=message@entry=0x23e1288 "!inMap && \"Duplicate threat list entry being inserted - memory leak!\"")

Current behaviour:
Believe related to threat system rewrite

(Right now crashing like hell)

• https://pastebin.com/X4H3PYwN
• https://pastebin.com/ga1NzjL4

Expected behaviour: Tell us what should happen instead

Steps to reproduce the problem:
Dont know

Branch(es): 3.3.5 / 6.x (Select the branch(es) affected by this issue)
3.3.5

TC hash/commit:
e0b609a178528995959e5ea54255a3f856e62913

TDB version:
335.63

Operating system:
Linux x64, Static

[Keader]

@Treeston

[Treeston]

Any idea what's happening just prior to the crash? Seems like UB somewhere, that assertion should be impossible to trigger.

[Shauren]

This assertion message is completely useless. You should change it to include some more info - don't forget that ASSERT accepts format arguments ASSERT(condition, format, args)

[Treeston]

Good point. @n4ndo, would you mind going to ~5a9fdda~ fdacf12 since your population seems to be able to reproduce this with some consistency? Hopefully that'll shed some light on what's going on here.

[n4ndo]

Sure no problem, let me just arrive home and i give feedback.

[Treeston]

@n4ndo what happened to further crash dumps?

[tje3d]

@Treeston No problem here for about 1 day ( with 30-40 players )

[Treeston]

Are you _sure_ the original crashes happened on e0b609a, and not some earlier rev?

[Treeston]

Are you sure the original crashes happened on e0b609a, and not some earlier rev?

@tje3d

[tje3d]

@Treeston Eveyrthing works fine, i have no problem for days.

[Treeston]

.....I will assume you posted the incorrect commit hash in the original issue then. Closing.

[n4ndo]

Hello @Treeston i'm sorry, i put the correct commit hash, since this pased days was holydays, i was forced to revert up to 425b181544a21d2246fdf0261ba76a37e2510883 (can't leave the server crashing, right now almost 3 days without crash).

I go to update up to 57672fdf259d1106420e6a2c91f6f1b65b00fbc9 and tell you the results, sorry my delay.

[Treeston]

Oh - apologies, confused you two. Re-opening until confirmation.

[n4ndo]

Ok, so, today i finally update to 274dede3dc89fa72ec1687d5ff454feba9849e47, and first crash:

Sources/src/server/game/Combat/ThreatManager.cpp:696 in PutThreatListRef ASSERTION FAILED:
  !inMap Duplicate threat reference at 0x7f538d790f40 being inserted on GUID Full: 0xf1300079aa000024 Type: Creature Entry: 31146  Low: 36 for GUID Full: 0xf1300079f0003099 Type: Creature Entry: 31216  Low: 12441 - memory leak!

[Treeston]

Thanks, very helpful - will take a look.

[n4ndo]

Another:

• https://pastebin.com/pnHqrGVf

[Treeston]

Can you post assertion output + crash dump for these?

[Treeston]

31216 is a target dummy...the plot thickens.

[Treeston]

~3dcd49e~ b53cbf4 fixes a super edge case iterator invalidation that could _potentially_ be the cause of this. Could you update and report back?

[n4ndo]

sure @Treeston let me update again and i tell you the results.

[Treeston]

@n4ndo have you updated to b53cbf4 yet? If yes, has this fixed the crashes?

[n4ndo]

Sorry @Treeston my delay, tomorrow i go to update and tell you the results.

[Foereaper]

Updated to latest rev 46e1e35, hitting the assert in ThreatManager::PutThreatListRef ThreatManager.cpp

Every time it has happened so far on my end is when a mage has used Mirror Image when attacking a target dummy, not sure if it is relevant or not though.

[Treeston]

That's very interesting, @Foereaper. I'll take a look at that specific case when I find the time.

[Foereaper]

Yeah, just checked all the dumps I have so far, and they all point to creature ID 31216, which is Mirror Image

http://prntscr.com/i00bl0

[Laintime]

@Treeston https://pastebin.com/WTTUChVv 5 duplicated crashes fo 6h ,uptime 1h.
rev https://github.com/TrinityCore/TrinityCore/commit/1c60af632888433b29a27bee76e82e96632d4096 clean tk (Unix, RelWithDebInfo, Static)

[Laintime]

@Treeston any news???it's critical fail

[Treeston]

Should have some time a bit later today.

[Treeston]

Bingo - with mirror images I can reproduce this.

_(or at least I was able to reproduce it once, and not since. heh.)_

[Treeston]

@Laintime @Foereaper @n4ndo 1847555 should fix the underlying issue here. Mirror Image AI is a tad naughty - but that's fine, and the core should handle it gracefully.

I couldn't reproduce it at all after this commit, so I believe this should fix the issue. Please report back.

[Laintime]

@Treeston thanks!

[Treeston]

@Laintime @Foereaper @n4ndo have any of you updated yet? Has this resolved the issue?

[Foereaper]

I have yet to get my test server back up and running, but will do so hopefully tomorrow, I'll let you know asap

[Laintime]

@Treeston I put only today,we wait.

[Treeston]

@Laintime @Foereaper @n4ndo any updates?

[Laintime]

@Treeston yes,two days uptime.
Thank you)

[Foereaper]

Seems good so far!

[Laintime]

@Treeston open issue https://pastebin.com/QGC8qEmu
rev https://github.com/TrinityCore/TrinityCore/commit/b8c281f497d5da2251b11ab09f182a23a5721a1c (TrinityCore branch) (Unix, RelWithDebInfo, Static)
crash :'(

[Jildor]

npc_pet_mage_mirror_image::npc_pet_mage_mirror_imageAI::InitializeAI (this=0x7fff2953f000) at /home/source/TrinityCore/src/server/scripts/Pet/pet_mage.cpp:126

        owner = <optimized out>

[Treeston]

f*ing mirror images, how do they work

[Treeston]

(but yeah, that's a release crashdump, not much info in there - any idea what caused it?)

[Foereaper]

No crash here so far, running debug though and will let you know if/when it happens

[Laintime]

@Treeston CombatManager::IsInCombatWith (this=0x7fff175ce790, guid=...) at /home/source/azur/src/server/game/Combat/CombatManager.cpp:210
210 return (_pveRefs.find(guid) != _pveRefs.end()) || (_pvpRefs.find(guid) != _pvpRefs.end());

No find guid,maybe summon ads fo boss,no guid target die,crash

[Langerz82]

have you got the call stack?

[Treeston]

what @Laintime posted is not a line that could ever crash in a debug context

[Laintime]

@Treeston Tomorrow I'll put the debug on my server
Now I'm on a live branch without the combat manager

[n4ndo]

Well i update today up to e45d54d80e3faf253d9f54d14866aacc8d019169 and 20 crash until now.

Now i shows Creature::SelectVictim():

• https://pastebin.com/fQ3PEU8Y
• https://pastebin.com/s9bKJ9iM
• https://pastebin.com/CdbXQ257

going to update to 8f71307e5619c8ef1801488812e20d07714fd733

Cordially

[Treeston]

Yeah, see if 1acfefb fixes this.

[Treeston]

@Laintime @Foereaper @n4ndo any updates since 1acfefb?

[Foereaper]

I haven't experienced anything at least

[Treeston]

assuming fixed