🐛 [Bug] Swiftmend crash fix

[Wall-core]

Swiftmend uses a jank blizzlike structure and has its own special function handling it. this function can crash if you somehow hit a race condition on the dispel mask. It can be fixed by checking the iterator is valid before checking spellproto.
In Unit.cpp:

bool Unit::HasAuraFamily(AuraType type, SpellFamily family, uint64 familyFlag) const
{
    AuraList const& auras = GetAurasByType(type);
    for (const auto& i : auras)
        if (i && i->GetSpellProto() && i->GetSpellProto()->IsFitToFamily(family, familyFlag))
            return true;

    return false;
}

[DeltaF1]

I'm not familiar with how concurrency works in this codebase. Can you elaborate on where the race condition is? Is the problem that the auras list could change and thus auras be destroyed between calling GetAurasByType and IsFitToFamily?

[Wall-core]

I haven't been able to pinpoint it precisely based on stacktrace but can confirm this fixed the rare crash occurring. It seemingly is related to rapidly tab-targeting units which are friendly/unfriendly while mashing swiftmend. Not sure how the iterator is becoming invalid before being called but it's related to the mask checks used previously.

[DeltaF1]

Could you post the rest of the code changes you made to fix it? I'm having a hard time figuring out where in the Swiftmend code this added method would be called.

How would you recommend reproducing this issue? You said to switch rapidly between targeting various units while spamming it. Should the friendly units have restoration/rejuvenation on them?

[Wall-core]

The only change made is validating the iterator:
if (i && i->GetSpellProto()

[DeltaF1]

Okay so that would be in here?

core/src/game/Spells/SpellEffects.cpp

Lines 2495 to 2503 in d58f8f6

	Unit::AuraList const& RejorRegr = unitTarget->GetAurasByType(SPELL_AURA_PERIODIC_HEAL);
	// find most short by duration
	Aura* targetAura = nullptr;
	for (const auto i : RejorRegr)
	{
	// Regrowth or Rejuvenation
	if (i->GetSpellProto()->IsFitToFamily<SPELLFAMILY_DRUID, CF_DRUID_REJUVENATION, CF_DRUID_REGROWTH>())
	if (!targetAura || i->GetAuraDuration() < targetAura->GetAuraDuration())
	targetAura = i;

I just want to make sure I'm getting it right since I couldn't find the function Unit::HasAuraFamily anywhere in the codebase.