[vJASS] Double free of type error

[pred1980]

Hallo hivers,

can someone help me by finding out why i get sometimes the error message "Double free of type: FleshWound__FleshWoundData". It's only sometimes...

Here's the code for the Spell:

scope FleshWound initializer init
    /*
     * Description: Every second hit weakens the enemys armor by 2 points for 5 seconds. 
	                The effect can stack several times.
     * Changelog: 
     *      28.10.2013: Abgleich mit OE und der Exceltabelle
	 *		13.04.2015: Code Refactoring
	 *					Integrated SpellHelper for filtering
	 *		31.01.2016: Reworked stack process (was necessary for working correctly in AI System)
	 *		07.02.2016: Bugfixing
     *
     */
    private keyword FleshWound
	private keyword FleshWoundData

    globals
        private constant integer SPELL_ID = 'A04T'
        private constant integer HITS = 2
		// This delay is for reseting the hitCounter + stackCounter if no stroke comes after the last one
		private constant real HIT_DELAY = 3.0
		// This duration describes how long the "-armor" works
        private constant real STACK_DURATION = 5.0
		// this duration is for the effect
		private constant real EFFECT_DURATION = 1.5
		private constant string EFFECT = "Abilities\\Spells\\Other\\HowlOfTerror\\HowlTarget.mdl"
        
		private integer array ARMOR_REDUCE
        private FleshWound array fleshWoundForCaster
		private FleshWoundData array fleshWoundForTarget
    endglobals
    
    private function MainSetup takes nothing returns nothing
		set ARMOR_REDUCE[1] = -2
        set ARMOR_REDUCE[2] = -4
        set ARMOR_REDUCE[3] = -6
        set ARMOR_REDUCE[4] = -8
        set ARMOR_REDUCE[5] = -10
    endfunction
	
	private struct FleshWoundData
		unit target
		integer hitCounter
		integer stackCounter
		timer hitTimer
		timer stackTimer
		boolean isHitInTime
		real time = 0.
		trigger t
		private static thistype tempthis = 0
		
		method onDestroy takes nothing returns nothing
			set fleshWoundForTarget[GetUnitId(.target)] = 0
			set .target = null
		endmethod
		
		private static method onUnitDeath takes nothing returns nothing
			local unit killedUnit = GetTriggerUnit()
			
			if (getForUnit(killedUnit) != 0) then
				call getForUnit(killedUnit).destroy()
			endif
			
			set killedUnit = null
		endmethod
		
		static method getForUnit takes unit u returns thistype
			return fleshWoundForTarget[GetUnitId(u)]
		endmethod
		
		static method onFleshWoundEnd takes nothing returns nothing
			local thistype data = GetTimerData(GetExpiredTimer())
           
			set data.time = data.time - 1.0
			if (data.time <= 0) then
				call ReleaseTimer(GetExpiredTimer())
				call SetUnitBonus(data.target, BONUS_ARMOR, 0)
			endif
		endmethod
		
		static method onHitReset takes nothing returns nothing
			local thistype data = GetTimerData(GetExpiredTimer())
			
			call ReleaseTimer(GetExpiredTimer())
			if (not data.isHitInTime) then
				set data.hitCounter = 0
				set data.stackCounter = 0
				set data.isHitInTime = false
			endif
		endmethod

		static method create takes unit damagedUnit returns thistype
			local thistype this = thistype.allocate()
			local trigger t = CreateTrigger()

			set fleshWoundForTarget[GetUnitId(damagedUnit)] = this
			
			call TriggerRegisterUnitEvent(t, damagedUnit, EVENT_UNIT_DEATH)
			call TriggerAddAction(t, function thistype.onUnitDeath)
			set t = null
			
			set .tempthis = this
			
			return this
		endmethod
	endstruct
    
    private struct FleshWound
		
		static method getForUnit takes unit u returns thistype
			return fleshWoundForCaster[GetUnitId(u)]
		endmethod

		method onAttack takes unit damageSource, unit damagedUnit, real dmg returns nothing
			local integer level = GetUnitAbilityLevel(damageSource, SPELL_ID)
			local FleshWoundData data = FleshWoundData.getForUnit(damagedUnit)

			if (data == 0) then
				set data = FleshWoundData.create(damagedUnit)
				set data.target = damagedUnit
				set data.hitCounter = 0
				set data.stackCounter = 0
			endif

			set data.hitCounter = data.hitCounter + 1
			set data.isHitInTime = true
			
			if (data.hitCounter == HITS) then
				set data.hitCounter = 0

				if (data.stackCounter <= level) then
					if (data.stackCounter < level) then
						set data.stackCounter = data.stackCounter + 1
					endif
					
					if (GetUnitBonus(data.target, BONUS_ARMOR) != ARMOR_REDUCE[data.stackCounter]) then
						call SetUnitBonus(data.target, BONUS_ARMOR, ARMOR_REDUCE[data.stackCounter])
						call TimedEffect.createOnUnit(EFFECT, data.target, "origin", EFFECT_DURATION)
					endif
					
					if (data.time <= 0.) then
						set data.time = STACK_DURATION
						set data.stackTimer = NewTimer()
						call SetTimerData(data.stackTimer, data)
						call TimerStart(data.stackTimer, 1.0, true, function FleshWoundData.onFleshWoundEnd)
					else
						set data.time = STACK_DURATION
					endif
				endif			
			endif
			
			if (TimerGetRemaining(data.hitTimer) <= 0.) then
				set data.hitTimer = NewTimer()
				call SetTimerData(data.hitTimer, data)
				call TimerStart(data.hitTimer, HIT_DELAY, false, function FleshWoundData.onHitReset)
				
				set data.isHitInTime = false
			endif
		endmethod

		static method create takes unit damageSource returns thistype
            local thistype this = thistype.allocate()

			set fleshWoundForCaster[GetUnitId(damageSource)] = this
			
			return this
        endmethod
    endstruct
	
	// damageSource == Ghoul
	// damagedUnit  == Target
    private function Actions takes unit damagedUnit, unit damageSource, real damage returns nothing
        local FleshWound fw = FleshWound.getForUnit(damageSource)
        
        if (GetUnitAbilityLevel(damageSource, SPELL_ID) > 0 	and /*
		*/	SpellHelper.isValidEnemy(damagedUnit, damageSource) and /*
		*/	DamageType == PHYSICAL ) then
			if (fw != 0) then
				call fw.onAttack(damageSource, damagedUnit, damage)
			endif
        endif
    endfunction
	
	private function SkillActions takes nothing returns nothing
		local unit caster = GetTriggerUnit()
		local FleshWound fw = FleshWound.getForUnit(caster)
		
		if (fw == 0) then
			set fw = FleshWound.create(caster)
		endif
		
        set caster = null
	endfunction
	
	//The condition if trigger should run (refers to the event : EVENT_PLAYER_HERO_SKILL)
    private function Conditions takes nothing returns boolean
        return GetUnitAbilityLevel(GetTriggerUnit(), SPELL_ID) != 0
    endfunction

    private function init takes nothing returns nothing
		call RegisterPlayerUnitEvent(EVENT_PLAYER_HERO_SKILL, function Conditions, function SkillActions)
        call RegisterDamageResponse( Actions )
		call MainSetup()
    endfunction

endscope

 

[BPower]

        private static thistype tempthis = 0
        
        method onDestroy takes nothing returns nothing
            set fleshWoundForTarget[GetUnitId(.target)] = 0
            set .target = null
        endmethod
        
        private static method onUnitDeath takes nothing returns nothing
            call .tempthis.destroy()
        endmethod

I guess that instead of .tempthis.destroy, you should do
getForUnit[GetTriggerUnit()].destroy()
Better if getForUnit[GetTriggerUnit()] != 0 then
getForUnit[GetTriggerUnit()].destroy()

 

[pred1980]

ok, I'll test it later.

as i understand you correct...

private static method onUnitDeath takes nothing returns nothing
    if (getForUnit[TriggerUnit] != 0) then
        getForUnit[TriggerUnit()].destroy()
    endif
endmethod

right?

 

[BPower]

yes that's correct.

 

[pred1980]

@BPower: It still happens - If you need the map to test urself, you can just download it from git (signature--> Source Code).
Start map, select the Ghoul Hero and choose the ability "Flesh Wound". Than play some minutes... The spell-code can be found at the git too.

Any other ideas?

 

[Anitarf]

This spell would benefit a lot from using some kind of a buff system. You could outsource a lot of the work you are doing to such a system, making things easier to manage and less likely to bug.

I don't know why you are still getting double frees, the error that BPower pointed out is the only one I can immediately see as well, unless you didn't fix it correctly, or you changed other things. You should update the code in the first post and/or on your site so we can see what kind of changes you made.

Just a comment on your use of BonusMod: the way you are using it, no other spell will be able to use it to modify armour, which is not ideal.

Edit: also, you really don't need to use dynamic triggers for this.

 

[edo494]

the thing is, it is deallocated, but the value is not set properly, so you should do

private static method onUnitDeath takes nothing returns nothing
    if (getForUnit[TriggerUnit] != 0) then
        call getForUnit[TriggerUnit()].destroy()
        set getForUnit[TriggerUnit()] = 0
    endif
endmethod

 

[Anitarf]

the thing is, it is deallocated, but the value is not set properly,

Actually, it does get set to 0 in the onDestroy method, so that should be fine.

 

[edo494]

well ok, didnt see the whole source, the problem then is imo that you are using onDestroy, and not destroy methods(Why would you ever consider using ondestroy?)

 

[Kazeon]

@BPower: It still happens - If you need the map to test urself, you can just download it from git (signature--> Source Code).
Start map, select the Ghoul Hero and choose the ability "Flesh Wound". Than play some minutes... The spell-code can be found at the git too.

Any other ideas?

How often does that happen? I suspect it happens for revived units:
- a unit is death => deallocate this, then revived and killed again => deallocate 0
- the same unit is revived and killed again => deallocate 0 again => double free
- (or) another unit is death => deallocate this, then revived and killed again => deallocate 0 again => double free

I haven't ever experimented with deallocating 0 tho. So I'm not sure is this really the issue. If so, then the solution is using unit indexer and deallocate only on deindex event, and don't forget to destroy the trigger you created on "create" method.

You can show debug message what index is being allocated and deallocated on allocation and deallocation and see what index is double-freed.

EDIT:
Tested. And deallocating 0 results another kind of error. Still, try to display the debug message I suggested before. It can probably help.

 

[pred1980]

1. I updated the spell code in the first post!
2. I tested it and atm i don't get any error message.

I'm not rly sure if its fixed ^^

@Anitarf:
Yeah, you're right, a buff system would be a better way.
BonusMod: How i must change it that other spells can modify the armor value?

@Quilnez
If the error came back, i'll add the debug messages.

 

[Anitarf]

Right now, you are setting the total bonus of the unit to that of the spell. This would overwrite any other bonuses applied by other spells. The solution is to modify the unit's bonus rather than setting it. So, when the armour reduction goes to the next level, you read from your array the values for both the current and the next level, then calculate the difference between them. Then you get the current armour bonus of the unit and increase it by that difference (or just use AddUnitBonus instead of SetUnitBonus). Then, when the spell ends, decrease the current armour bonus of the unit by the amount stored for the current level, rather than just setting it to 0.

 

[pred1980]

Can u show how i have to change it?

 

[BPower]

Can u show how i have to change it?

It's in the text, well explained.

(or just use AddUnitBonus instead of SetUnitBonus)

 

[pred1980]

It's in the text, well explained.

ah, sry... my mistake ^^. I'll change it!