[crash] SpellAuraHolder

Toinan67 · August 9, 2010

Aura is removed, and that's the "problem" actually no?

We have an instant crash, not a freeze/infinite loop (see The_Game_Master's crash logs)

The aura is removed from the target but not from the slowingAuras list, then the iterator returns to slowingAuras.begin()

And when we reach the aura we just removed...

Fails here :

SpellEntry const* aurSpellInfo = (*iter)->GetSpellProto();
uint32 aurMechMask = GetAllSpellMechanicMask(aurSpellInfo);

SpellMgr.h :

inline uint32 GetAllSpellMechanicMask(SpellEntry const* spellInfo)
{
   uint32 mask = 0;
   if (spellInfo->Mechanic)
       mask |= 1 << (spellInfo->Mechanic - 1);
   for (int i=0; i< 3; ++i)
       if (spellInfo->EffectMechanic[i])
           mask |= 1 << (spellInfo->EffectMechanic[i]-1);
   return mask;
}

ASSERT(spellInfo); would have been nice

Am I totally wrong here? x)

lavinelu · August 9, 2010

i think that when aura is removed it should be remove form slowingAuras list too because this is a pointer or maybe i am wrong

Dron01 · August 9, 2010

So it's not needed

Zergtmn, what did you mean? Does reference automatically remove pointers to its elements when they are deleted? Anyway, this code worked since some "4-digits" revisions, and I'm in doubts...

+    for (Unit::AuraList::const_iterator iter = target->GetAurasByType(SPELL_AURA_MOD_DECREASE_SPEED).begin(); iter != target->GetAurasByType(SPELL_AURA_MOD_DECREASE_SPEED).end()

Isn't it too slow?.. Every loop execution it gets an array of auras and then checks it, even if aura is not deleted!

Unit::AuraList &slowingAuras = (Unit::AuraList&)(target->GetAurasByType(SPELL_AURA_MOD_DECREASE_SPEED))

Is this safe? If yes, we could delete elements right in the loop...

Toinan67 · August 9, 2010

lavinelu > I think it's not removed from slowingAuras

itr is a reference to a const AuraList. So the list does not change... (or is it the reference which is const?)

Haha I'm not sure anymore

Help needed

Dron01 > For sure it's slower but I don't see any other way for the moment

And see the prototype of GetAurasByType :

AuraList const& GetAurasByType(AuraType type) const { return m_modAuras[type]; }

You'll have an invalid conversion from AuraList const& to Auralist & or something like that

lavinelu · August 9, 2010

lavinelu > I think it's not removed from slowingAuras

it's not removed but it should be removed when you call this function target->RemoveAurasDueToSpellByCancel(aurSpellInfo->Id);

ankso · August 9, 2010

should be checked why this aura is not removed, this code is ok how it is right now.

So the problem is in RemoveAurasDueToSpellByCancel ?

Dron01 · August 9, 2010

He-he, of course it was right. I thought GetAurasByType forms something like array of pointers to those auras and then returns a pointer to its beginning, didn't thought it could be static array. So aura should be deleted from m_modAuras[type] (which is just a list of pointers with complicated name ) and it does NOT. But aura itself is deleted and deallocated really so the pointer stored in this list becomes invalid. Anyway, we are close...

Toinan67 · August 9, 2010

I was wrong, lavinelu is correct : problem is (I think) from RemoveAurasDueToSpellByCancel

The aura is not removed

void Unit::RemoveAurasDueToSpellByCancel(uint32 spellId)
{
   SpellAuraHolderBounds spair = GetSpellAuraHolderBounds(spellId);
   for(SpellAuraHolderMap::iterator iter = spair.first; iter != spair.second;)
   {
       RemoveSpellAuraHolder(iter->second, AURA_REMOVE_BY_CANCEL);
       spair = GetSpellAuraHolderBounds(spellId);
       iter = spair.first;
   }
}

Dron01 · August 9, 2010

SpellAuraHolder::~SpellAuraHolder()
{
   // note: auras in delete list won't be affected since they clear themselves from holder when adding to deletedAuraslist
   for (int32 i = 0; i < MAX_EFFECT_INDEX; ++i)
       if (Aura *aur = m_auras[i])
           delete aur;
}

can it be that somewhy holder deletes itself and its auras without clearing itself out from arrays?

The_Game_Master · August 9, 2010

If anybody want to use a dirty hack as a temp solution, you can use this. But the pointer is lost way before this function. I haven't been able to find it, but looks like you guys made some real progress.

+                // SpellEntry const* aurSpellInfo = (*iter)->GetSpellProto();
+                SpellEntry const* aurSpellInfo;
+                if (aurSpellInfo = (*iter)->GetSpellProto())
+                    break;

vipertv · August 9, 2010

Or maybe not so dirty hack. But as i think, the fix is in the place where is the real reason of problem.

In GridNotifiersImpl.h in function void MaNGOS::DynamicObjectUpdater::VisitHelper(Unit* target)

  // Apply PersistentAreaAura on target

   SpellAuraHolder *holder = target->GetSpellAuraHolder(spellInfo->Id, i_dynobject.GetCaster()->GetGUID());

   bool addedToExisting = true;
   if (!holder)
   {
       holder = CreateSpellAuraHolder(spellInfo, target, i_dynobject.GetCaster());
       addedToExisting = false;

   }

+    if (holder->GetAuraByEffectIndex(eff_index))
+            target->RemoveAura(holder->GetAuraByEffectIndex(eff_index), AURA_REMOVE_BY_STACK);
+
   PersistentAreaAura* Aur = new PersistentAreaAura(spellInfo, eff_index, NULL, holder, target, i_dynobject.GetCaster());
   holder->AddAura(Aur, eff_index);

   if (addedToExisting)
   {
       target->AddAuraToModList(Aur);
       holder->SetInUse(true);
       Aur->ApplyModifier(true,true);
       holder->SetInUse(false);
   }
   else
       target->AddSpellAuraHolder(holder);

lavinelu · August 9, 2010

i did it in this way but i dont know how the spell system is working and maybe will broke other spells

i think the same spell should not be added 2 times here target->AddAuraToModList(Aur);

@@ -4116,11 +4116,22 @@ bool Unit::AddSpellAuraHolder(SpellAuraHolder *holder)
}

void Unit::AddAuraToModList(Aura *aura)
{
    if (aura->GetModifier()->m_auraname < TOTAL_AURAS)
+    {
+        AuraList::const_iterator iter;
+        for (iter = m_modAuras[aura->GetModifier()->m_auraname].begin(); iter != m_modAuras[aura->GetModifier()->m_auraname].end(); ++iter)
+        {
+            if (aura->GetId() == (*iter)->GetId())
+            {
+                m_modAuras[aura->GetModifier()->m_auraname].remove((*iter));
+                break;
+            }
+        }
        m_modAuras[aura->GetModifier()->m_auraname].push_back(aura);
+    }
}

vipertv · August 9, 2010

And maybe the real fix will be, when we add to holder and to m_modAuras a new aura only when it's needed.

Vinolentus · August 9, 2010

can it be that somewhy holder deletes itself and its auras without clearing itself out from arrays?

Sad but true.

Holder created : 0x72646640

Holder deleted : 0x72646640

...

Unit::GetAura:

(i*)->m_spellAuraHolder : 0x72646640

Dron01 · August 9, 2010

Sad but true.
Holder created : 0x72646640

Holder deleted : 0x72646640

...

Unit::GetAura:

(i*)->m_spellAuraHolder : 0x72646640

And shine Aurora now. Specific bug with persistent auras, I guess...

Looking for a solution, hmm ... We should move insertion in m_modAuras to aura constructor and deletion from it to aura destructor.

It will break original idea partially but this will do the trick, I think.

Another way is to look through code to find why persistent aura holders get deleted before its components. There's nothing else to loose, maybe we should add ASSERT on aura deletion to the holder destructor?

Then we can see the stacktrace at least...

Lynx3d · August 9, 2010

Or maybe not so dirty hack. But as i think, the fix is in the place where is the real reason of problem.
In GridNotifiersImpl.h in function void MaNGOS::DynamicObjectUpdater::VisitHelper(Unit* target)

For the freeze with Desecration + Shapeshift, this is definitely the place where it goes wrong yes.

It has nothing to do with AuraHolder functions going wrong, problem is that multiple DynamicObjects from different effects of same spell have to share SpellAuraHolder in current code design, so this manual fiddling is necessary for now.

You cannot just delete the whole holder, because if spell have more than one effect, second effect will erase first effect etc...

Fix for this specific problem added in [10335]

The_Game_Master · August 10, 2010

Should i update the remaining problems or open a new subject?

Dron01 · August 10, 2010

Ofc we should recheck it...

Toinan67 · August 10, 2010

The_Game_Master I think updating the remaining problems is the best, otherwise we'll have 2 threads for 1 problem...might be messy

lavinelu · August 10, 2010

For the freeze with Desecration + Shapeshift, this is definitely the place where it goes wrong yes.
It has nothing to do with AuraHolder functions going wrong, problem is that multiple DynamicObjects from different effects of same spell have to share SpellAuraHolder in current code design, so this manual fiddling is necessary for now.

You cannot just delete the whole holder, because if spell have more than one effect, second effect will erase first effect etc...

Fix for this specific problem added in [10335]

Lunx3d, here target->AddAuraToModList(Aur); it's ok to have same aura added 2 times or more?

if not can something go wrong if i modify it like this?

@@ -4116,11 +4116,22 @@ bool Unit::AddSpellAuraHolder(SpellAuraHolder *holder)
}

void Unit::AddAuraToModList(Aura *aura)
{
    if (aura->GetModifier()->m_auraname < TOTAL_AURAS)
+    {
+        AuraList::const_iterator iter;
+        for (iter = m_modAuras[aura->GetModifier()->m_auraname].begin(); iter != m_modAuras[aura->GetModifier()->m_auraname].end(); ++iter)
+        {
+            if (aura->GetId() == (*iter)->GetId())
+            {
+                m_modAuras[aura->GetModifier()->m_auraname].remove((*iter));
+                break;
+            }
+        }
        m_modAuras[aura->GetModifier()->m_auraname].push_back(aura);
+    }
}

Lynx3d · August 10, 2010

Lunx3d, here target->AddAuraToModList(Aur); it's ok to have same aura added 2 times or more?

Why not?

I really don't it's the task of this function to decide whether auras stack or not.

Basically, this is a very low-level function, and using it directly should be avoided wherever possible. And there's infact only two places outside AddSpellAuraHolder() where it is used, the one just fixed, and Creature::LoadCreaturesAddon() which might need the same fix actually...

lavinelu · August 10, 2010

Why not?
I really don't it's the task of this function to decide whether auras stack or not.

Basically, this is a very low-level function, and using it directly should be avoided wherever possible. And there's infact only two places outside AddSpellAuraHolder() where it is used, the one just fixed, and Creature::LoadCreaturesAddon() which might need the same fix actually...

thanks for answer, i asked because i put this modification on server and now uptime is 16h, didn't crashed since then and before every 1 - 2 hours was a crash.

VladimirMangos · August 11, 2010

Is all listed problems fixed now? Or can be clarify what from not fixed yet?

shinra · August 11, 2010

Another crash, this is no way fixed by the way im crashing all the time related or auras.

Debug mode is pointing to here

Spell Auras

// re-apply passive spells that don't need shapeshift but were inactive in current form:

const PlayerSpellMap& sp_list = ((Player *)target)->GetSpellMap();

>> for (PlayerSpellMap::const_iterator itr = sp_list.begin(); itr != sp_list.end(); ++itr)

> mangosd.exe!Aura::HandleShapeshiftBoosts(bool apply) Line 6049 + 0x6 bytes C++

Revision: * * 10345 4a181802416927280fdbbbef625c6403757c42f4
Date 11:8:2010. Time 23:15 
//=====================================================
*** Hardware ***
Processor: Quad-Core AMD Opteron(tm) Processor 1352
Number Of Processors: 4
Physical Memory: 4194303 KB (Available: 4194303 KB)
Commit Charge Limit: 4194303 KB

*** Operation System ***
Windows Vista or Windows Server 2008 Server 4.0 (Version 6.1, Build 7600)

//=====================================================
Exception code: C0000005 ACCESS_VIOLATION
Fault address:  00644F82 01:00243F82 E:\\CompiledServer\\mangosd.exe

Registers:
EAX:00000000
EBX:DDF198A0
ECX:E17175B0
EDX:E17175B0
ESI:E16EE080
EDI:00001CD0
CS:EIP:0023:00644F82
SS:ESP:002B:0A64AB4C  EBP:0A64AB70
DS:002B  ES:002B  FS:0053  GS:002B
Flags:00010246

Call stack:
Address   Frame     Function      SourceFile
00644F82  00000000  Aura::HandleShapeshiftBoosts+512
00645B16  00000000  Aura::HandleAuraModShapeshift+4B6
0063EEA2  00000000  Aura::ApplyModifier+32
00498F6E  00000000  Unit::RemoveAura+9E
0049D944  00000000  Unit::RemoveSpellAuraHolder+144
0049DC93  00000000  Unit::RemoveAllAurasOnDeath+23
004A429E  00000000  Unit::setDeathState+4E
004B7213  00000000  Creature::setDeathState+B3
004A66F1  00000000  Unit::DealDamage+831
004A7166  00000000  Unit::DealSpellDamage+F6
006E71B4  00000000  Spell::DoAllEffectOnTarget+3B4
006E74CB  00000000  Spell::handle_immediate+7B
006F0AF1  00000000  Spell::cast+571
006F166D  00000000  Spell::update+2DD
006F176D  00000000  SpellEvent::Execute+1D
007C3A28  00000000  EventProcessor::Update+58
004A56C0  00000000  Unit::Update+20
0054A6AE  00000000  Player::Update+6E
004DDE5E  00000000  Map::Update+5E
00627CD4  00000000  MapManager::Update+64
0046665F  00000000  World::Update+24F
004462E4  00000000  WorldRunnable::run+64
007D3FF4  00000000  ACE_Based::Thread::ThreadTask+34
644E5B04  00000000  ACE_OS_Thread_Adapter::invoke+74
735CC6DE  00000000  _endthreadex+3A
735CC788  00000000  _endthreadex+E4
752A3677  00000000  BaseThreadInitThunk+12
77699D72  00000000  RtlInitializeExceptionChain+63
77699D45  00000000  RtlInitializeExceptionChain+36
========================
Local Variables And Parameters

Call stack:
Address   Frame     Function      SourceFile
00644F82  00000000  Aura::HandleShapeshiftBoosts+512
punting on symbol apply
punting on symbol spellId1
punting on symbol form
punting on symbol spellId2
punting on symbol HotWSpellId
punting on symbol MasterShaperSpellId
   Local  <user defined> 'itr'
punting on symbol ShiftMod
punting on symbol HotWMod

00645B16  00000000  Aura::HandleAuraModShapeshift+4B6
punting on symbol apply
punting on symbol Real
   Local  <user defined> 'form'
punting on symbol modelid
   Local  <user defined> 'ssEntry'
   Local  <user defined> 'PowerType'
punting on symbol furorChance

0063EEA2  00000000  Aura::ApplyModifier+32
punting on symbol apply
punting on symbol Real

00498F6E  00000000  Unit::RemoveAura+9E
   Local  <user defined> 'Aur'
   Local  <user defined> 'mode'

0049D944  00000000  Unit::RemoveSpellAuraHolder+144
   Local  <user defined> 'holder'
   Local  <user defined> 'mode'
   Local  <user defined> 'AurSpellInfo'
   Local  <user defined> 'caster'
   Local  <user defined> 'bounds'
   Local  <user defined> 'statue'

0049DC93  00000000  Unit::RemoveAllAurasOnDeath+23

004A429E  00000000  Unit::setDeathState+4E
   Local  <user defined> 's'

004B7213  00000000  Creature::setDeathState+B3
   Local  <user defined> 's'

004A66F1  00000000  Unit::DealDamage+831
   Local  <user defined> 'pVictim'
punting on symbol damage
   Local  <user defined> 'cleanDamage'
   Local  <user defined> 'damagetype'
   Local  <user defined> 'damageSchoolMask'
   Local  <user defined> 'spellProto'
punting on symbol durabilityLoss
punting on symbol duel_hasEnded
punting on symbol health
   Local  <user defined> 'spiritOfRedemtionTalentReady'
   Local  <user defined> 'player_tap'
   Local  <user defined> 'pOwner'
punting on symbol damageFromSpiritOfRedemtionTalent
   Local  <user defined> 'data'
   Local  <user defined> 'data'
   Local  <user defined> 'st'
   Local  <user defined> 'st'
   Local  <user defined> 'next'
   Local  <user defined> 'st'
   Local  <user defined> 'st'

004A7166  00000000  Unit::DealSpellDamage+F6
   Local  <user defined> 'damageInfo'
punting on symbol durabilityLoss
   Local  <user defined> 'spellProto'
   Local  <user defined> 'cleanDamage'

006E71B4  00000000  Spell::DoAllEffectOnTarget+3B4
   Local  <user defined> 'target'
   Local  <user defined> 'unit'
punting on symbol procEx
   Local  <user defined> 'caster'
   Local  <user defined> 'missInfo'
punting on symbol mask
punting on symbol procVictim
   Local  <user defined> 'real_caster'
punting on symbol procAttacker
punting on symbol crit
punting on symbol gain
   Local  <user defined> 'damageInfo'
punting on symbol count
   Local  <user defined> 'itr'
punting on symbol bp
   Local  <user defined> 'damageInfo'

006E74CB  00000000  Spell::handle_immediate+7B
punting on symbol duration

006F0AF1  00000000  Spell::cast+571
punting on symbol skipCheck

006F166D  00000000  Spell::update+2DD
punting on symbol difftime
   Local  <user defined> 'p'

006F176D  00000000  SpellEvent::Execute+1D
punting on symbol e_time
punting on symbol p_time

007C3A28  00000000  EventProcessor::Update+58
punting on symbol p_time

004A56C0  00000000  Unit::Update+20
punting on symbol p_time

0054A6AE  00000000  Player::Update+6E
punting on symbol p_time
punting on symbol now
   Local  <user defined> 'iter'
punting on symbol newzone
punting on symbol newarea

004DDE5E  00000000  Map::Update+5E
   Local  <user defined> 't_diff'
   Local  <user defined> 'updater'
   Local  <user defined> 'world_object_update'
   Local  <user defined> 'grid_object_update'
   Local  <user defined> 'area'
   Local  <user defined> 'begin_cell'
   Local  <user defined> 'end_cell'
punting on symbol x
   Local  <user defined> 'cell'
   Local  <user defined> 'pair'
   Local  <user defined> 'begin_cell'
   Local  <user defined> 'end_cell'
punting on symbol x
   Local  <user defined> 'cell'
   Local  <user defined> 'pair'
   Local  <user defined> 'st'

00627CD4  00000000  MapManager::Update+64
punting on symbol diff

0046665F  00000000  World::Update+24F
punting on symbol diff
punting on symbol maxClientsNum

004462E4  00000000  WorldRunnable::run+64
punting on symbol prevSleepTime

007D3FF4  00000000  ACE_Based::Thread::ThreadTask+34
punting on symbol param

644E5B04  00000000  ACE_OS_Thread_Adapter::invoke+74
punting on symbol status

735CC6DE  00000000  _endthreadex+3A

735CC788  00000000  _endthreadex+E4

752A3677  00000000  BaseThreadInitThunk+12

77699D72  00000000  RtlInitializeExceptionChain+63

77699D45  00000000  RtlInitializeExceptionChain+36

========================
Global Variables

Lynx3d · August 11, 2010

Err sorry, that one was a mistake in a commit i just did, should be fixed now...just a stupid oversight.

Retry with [10348]+ please...

Sign In

[crash] SpellAuraHolder

Recommended Posts

Link to comment

Share on other sites

Top Posters In This Topic

Popular Days

Top Posters In This Topic

Popular Days

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Recently Browsing 0 members

Contact Us

Repositories

Important Information