Things That Leak

[MyPad]

I would say that creating conditions in GUI is technically not a leak, but the group itself does if used momentarily, and it is not destroyed.

The method above is the recommended way of filtering out the desired units, and I think it is one out of two ways of filtering them out.

 

[Map Designer]

Lets have a look at this little trigger.

• Hard Extra Defense
  • Events
  • Conditions
  • Actions
• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
• If - Conditions
  • (Number of living Grunt units owned by Player 1 (Red)) Less than 4
  • (Number of living Barracks units owned by Player 1 (Red)) Greater than 0
• Then - Actions

Now to fix the leaking trigger above, I will write it like this:

• Hard Extra Defense
  • Events
  • Conditions
  • Actions
    • Set GroupTemp01 = (Units owned by Player 1 (Red) matching (((Matching unit) is alive) Equal to True))
    • Set GroupTemp02 = (Units owned by Player 1 (Red) matching (((Matching unit) is alive) Equal to True))
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Number of units in GroupTemp01) Less than 4
        • (Number of units in GroupTemp02) Greater than 0
      • Then - Actions
    • Custom script: call DestroyGroup(udg_GroupTemp01)
    • Custom script: call DestroyGroup(udg_GroupTemp02)

is that it? or what I did was totally unnecessary anyway?

@MyPad Post confused me
so it leaks or not?

 

[MyPad]

The first example will leak. However, the second will not leak objects, since you destroy the group handles.

Condition or no condition, creating a group means that you have to have the responsibility of destroying it.

Extra

If you'd like to get more technical, there is a bug that causes the handle stack to permanently increase, despite the successful elimination of handle objects (in this case, groups). It presents itself when a locally declared variable is passed as a return value in a function. Notwithstanding, the functions in GUI about unit groups all return the locally declared group handle.

It was present before, and might still be present now.

 

[Pyrogasm]

@map designer you can also automatically clean the group leak by doing Custom script: set bj_wantDestroyGroup = true right before the line that creates the group. That line tells the game to automatically destroy the next group it makes once it's done with it.

Unfortunately that will not work when you have 2 different groups created in the same if-block, since you can't put CS lines in there in GUI. It will work for just one, though. Also don't will-nilly set that variable unless you know FOR SURE you are about to create a group, since setting it and not making a group would cause the game to destroy whichever group it next makes, likely in a totally unrelated trigger.

 

[Map Designer]

@map designer you can also automatically clean the group leak by doing Custom script: set bj_wantDestroyGroup = true right before the line that creates the group. That line tells the game to automatically destroy the next group it makes once it's done with it.

Unfortunately that will not work when you have 2 different groups created in the same if-block, since you can't put CS lines in there in GUI. It will work for just one, though. Also don't will-nilly set that variable unless you know FOR SURE you are about to create a group, since setting it and not making a group would cause the game to destroy whichever group it next makes, likely in a totally unrelated trigger.

hmmmm

I thought that dynamic function will automatically destroy any group created by this trigger at the end of this trigger.

this raised the question for me though: when does this function destroy the group? at the end of this trigger? or when? if it was destroyed pre-maturely it would miss up the trigger wouldnt it?

 

[Pyrogasm]

Every unit group action uses ForGroupBJ() internally. The destroy happens right at the end of the function call:

function ForGroupBJ takes group whichGroup, code callback returns nothing
   // If the user wants the group destroyed, remember that fact and clear
   // the flag, in case it is used again in the callback.
   local boolean wantDestroy = bj_wantDestroyGroup
   set bj_wantDestroyGroup = false

   call ForGroup(whichGroup, callback)

   // If the user wants the group destroyed, do so now.
   if (wantDestroy) then
       call DestroyGroup(whichGroup)
   endif
endfunction

The way it stores the value of bj_wantDestroyGroup locally before calling ForGroup() itself allows you to nest group picks and they'll all be properly destroyed:

• Custom script: set bj_wantDestroyGroup = true
• Unit Group - Pick every unit in...
  • Loop - Actions
    • Custom script: set bj_wantDestroyGroup = true
    • Unit Group - Pick every unit in...
      • Loop - Actions
        • Custom script: set bj_wantDestroyGroup = true
        • Unit Group - Pick every unit in...
          • Loop - Actions
            • -------- ad infinitum --------

 

[JC Helas]

Sir can I use same key in each variable types in hashing to one parent and table?

 

[Bribe]

I highly discourage that. I recommend only storing one variable per parent/child key combination. If there is any data to show what I'm saying can be disregarded (ie. if they have done any tests) feel free to show me up here.

 

[JC Helas]

I highly discourage that. I recommend only storing one variable per parent/child key combination. If there is any data to show what I'm saying can be disregarded (ie. if they have done any tests) feel free to show me up here.

what I mean sir is,, I thought key is like an array of a variable isnt it?

 

[Bribe]

what I mean sir is,, I thought key is like an array of a variable isnt it?

There's a parent key, and a child key. Each parent represents an address which can store child keys with indices as high as integers allow.

So really, each parent key is a single array with "unlimited" slots but one which doesn't need to initialize all previous array indices in between slots. That's the difference between a hash table and an array.

So really I just want to make sure we're on the same page, cause you're not making too much sense so far.

 

[DracoL1ch]

I highly discourage that. I recommend only storing one variable per parent/child key combination. If there is any data to show what I'm saying can be disregarded (ie. if they have done any tests) feel free to show me up here.

actually you'd better reduce amount of parents to lowest possible, thats overheat right there. Better to have Save*(HT,0,spellid,callback) rather than Save*(HT,spellid,0,callback)

 

[Bribe]

actually you'd better reduce amount of parents to lowest possible, thats overheat right there. Better to have Save*(HT,0,spellid,callback) rather than Save*(HT,spellid,0,callback)

Good info there to add for him. Though I still would like to know what he's talking about.

 

[WaterKnight]

Sir can I use same key in each variable types in hashing to one parent and table?

At least handles are not disjunct. There is still the return bug substitute floating around where you can store an integer as fogstate and load it as another handle type. There is also the RemoveSavedHandle native, which hints at that. But often, you can limit yourself to storing integer indexes in hashtables and then use them in arrays. Or you could just use multiple hashtables.

actually you'd better reduce amount of parents to lowest possible, thats overheat right there. Better to have Save*(HT,0,spellid,callback) rather than Save*(HT,spellid,0,callback)

How so?

Good info there to add for him. Though I still would like to know what he's talking about.

JC Helas asked if e.g. SaveInteger(table1, parentKey1, key1, <integer>) and SaveUnitHandle(table2, parentKey2, key2, <unit>) where table1==table2, parentKey1==parentKey2, key1==key2 is viable, probably if they are disjunct as I responded to.

 

[DracoL1ch]

Maybe technicall they are identical in terms of storage usage, but remember one patch where blizzard broke hashtables by limiting parents count to 1024 or so? Within engine parents are being counted and stored, RemoveSavedX doesn't deduct parent counter, FlushChildHashtable() does although. I dont know where this counter is used and what purpose he serves, but why would you increase this counter with no good reason to? Thats my logic.

 

[JC Helas]

What if I will do this way

unit u = dummy
integer i = 1
real r = 1.00
integer id = GetHandleId(u)
SaveInteger(table,id,0,i)
SaveReal(table,id,0,r)

look I use 0 key in different native. is this leak?

 

[WaterKnight]

Maybe technicall they are identical in terms of storage usage, but remember one patch where blizzard broke hashtables by limiting parents count to 1024 or so? Within engine parents are being counted and stored, RemoveSavedX doesn't deduct parent counter, FlushChildHashtable() does although. I dont know where this counter is used and what purpose he serves, but why would you increase this counter with no good reason to? Thats my logic.

Never heard of it. I have no insight of the internals. When you say there's a counter, that seems marginal in terms of performance. My idea is to discriminate it by the FlushChildHashtable function, children are mapped to a parent, and this function makes it easy to erase everything in one swoop. Is the limit still in existence?

 

[DracoL1ch]

the counter itself means nothing, but as I said there might be inner stuff bound to it. HT is already linked list, MAYBE this counter used for something else what matter, maybe not. This arg swap costs 0 time for mapmaker so should be respected as long as it fits codestyle

 

[DracoL1ch]

What if I will do this way

unit u = dummy
integer i = 1
real r = 1.00
integer id = GetHandleId(u)
SaveInteger(table,id,0,i)
SaveReal(table,id,0,r)

look I use 0 key in different native. is this leak?

read documentary, hashtable can save 1 type per every cell, int/real/bool/handle/string, no problems. only handles can leak of those

 

[Gottfrei]

hey, guys. need to know is this function leaks "uC" cause i'm not nullifying it?

function GroupGetClosestUnit takes unit whichUnit, group whichGroup returns unit
    local integer i = 0
    local unit u = FirstOfGroup(whichGroup)
    local unit uC
    local unit array uG
    local real x = GetUnitX(whichUnit)
    local real y = GetUnitY(whichUnit)
    local real xT = GetUnitX(u)
    local real yT = GetUnitY(u)
    local real dist
    local real distC = SquareRoot((x - xT) * (x - xT) + (y - yT) * (y - yT))
    loop
        set u = FirstOfGroup(whichGroup)
        exitwhen u == null
        set xT = GetUnitX(u)
        set yT = GetUnitY(u)
        set dist = SquareRoot((x - xT) * (x - xT) + (y - yT) * (y - yT))
        if dist < distC then
            set uC = u
        endif
        set uG[i] = u
        set i = i + 1
        call GroupRemoveUnit(whichGroup, u)
    endloop
    loop
        exitwhen i < 0
        call GroupAddUnit(whichGroup, uG[i])
        set uG[i] = null
        set i = i - 1
    endloop
    set u = null
    return uC
endfunction

 

[Dr Super Good]

hey, guys. need to know is this function leaks "uC" cause i'm not nullifying it?

Yes it causes the handle index referenced by uC at the time of return to have a leaked reference count and hence when that unit is removed the handle index assigned to it cannot ever be recycled.

Solution is to use either a function parameter or a global variable to temporarily hold the value while the local is nulled and then return the value.

 

[Gottfrei]

Yes it causes the handle index referenced by uC at the time of return to have a leaked reference count and hence when that unit is removed the handle index assigned to it cannot ever be recycled.

Solution is to use either a function parameter or a global variable to temporarily hold the value while the local is nulled and then return the value.

so now its basically fixed? i can simply make uTemp parameter equal to "null".

function GroupGetClosestUnit takes unit whichUnit, group whichGroup, unit uTemp returns unit
    local integer i = 0
    local unit u
    local unit uC = FirstOfGroup(whichGroup)
    local unit array uG
    local real x = GetUnitX(whichUnit)
    local real y = GetUnitY(whichUnit)
    local real xT = GetUnitX(uC)
    local real yT = GetUnitY(uC)
    local real dist
    local real distC = SquareRoot((x - xT) * (x - xT) + (y - yT) * (y - yT))
    loop
        set u = FirstOfGroup(whichGroup)
        exitwhen u == null
        set xT = GetUnitX(u)
        set yT = GetUnitY(u)
        set dist = SquareRoot((x - xT) * (x - xT) + (y - yT) * (y - yT))
        if dist < distC then
            set uC = u
        endif
        set uG[i] = u
        set i = i + 1
        call GroupRemoveUnit(whichGroup, u)
    endloop
    loop
        exitwhen i < 0
        call GroupAddUnit(whichGroup, uG[i])
        set uG[i] = null
        set i = i - 1
    endloop
    set uTemp = uC
    set u = null
    set uC = null
    return uTemp
endfunction

 

[IcemanBo]

Not sure the extra parameter would be worth the ref leak, but yes, you're correct.

 

[Gottfrei]

Not sure the extra parameter would be worth the ref leak, but yes, you're correct.

and what would be better? global variable solution?

 

[IcemanBo]

You for sure can use it like this, I just would personally not like to give an extra parameter for counter a ref leak. As usage seems becomes worse.

Using a global would be maybe nicer, as it would change nothing for code usage. Some use Blizzard globals like bj_lastCreatedUnit, but using a seperated variable is better, as it can't come unwilligly in conflict with other's code. (if some GUI user in the example would ever work with "last created unit")

But that's also not really a "perfect" solution maybe. You also might to consider risking the leak, if you're too lazy for the global. ^^

 

[Gottfrei]

duh. this jass is killing me.

 

[Dr Super Good]

Use a global variable. Hopefully with reforged this leak finally gets fixed but for now I strongly advise explicitly fixing them to avoid issues in the meantime.

 

[Map Designer]

Hello,
this conition for trigger will leak right?
Event-
Unit owned by player 11 dies.
Conditions -
(Number of units in (Units owned by Player 11 (Dark Green))) Equal to 0

what about this condition
(Count structures controlled by Player 12 (Brown) (Exclude incomplete structures)) Equal to 0


and to fix it, I better move this condition into an if statement right? i.e. set Q equal number of units owned by etc

 

[IcemanBo]

Yes, in detail, "(Units owned by Player 11 (Dark Green))" does create a unit group.

And also yes, to prevent the leak in GUI you need to move this condition into the actions-block, and destroy the group as usual, with help of a variable.

 

[Dr Super Good]

Hello,
this conition for trigger will leak right?
Event-
Unit owned by player 11 dies.
Conditions -
(Number of units in (Units owned by Player 11 (Dark Green))) Equal to 0

Yes it leaks a group and even if that leak were fixed it will leak a handle ID.

and to fix it, I better move this condition into an if statement right? i.e. set Q equal number of units owned by etc

To fix the group leak you need to do what IcemanBo describes and move the test into an if block in the actions. This allows one to assign a unit group variable before performing the test and use DestroyGroup on it after the test.

Fixing the handle ID leak is harder. Requiring either a custom script call to a fixed version of "Units owned by" function or importing a fixed blizzard.j file. Importing a fixed blizzard.j file is not recommended as it might negatively effect a maps forwards compatibility with future versions of Warcraft III if Blizzard changes anything in that file.

what about this condition
(Count structures controlled by Player 12 (Brown) (Exclude incomplete structures)) Equal to 0

This does not leak as far as I can tell.

 

[Map Designer]

is (angle between 2 points) leaking?

or in general, does any arithmetic operation leak if they are not stored into a variable which then will be destroyed?

 

[Bribe]

is (angle between 2 points) leaking?

or in general, does any arithmetic operation leak if they are not stored into a variable which then will be destroyed?

That leaks 3 locations. Check the "Triggering tips" link in my signature for more information.

 

[Map Designer]

That leaks 3 locations. Check the "Triggering tips" link in my signature for more information.

sorry,

I meant if I saved the 2 points in a variable already, I will destroy these two points later. again, what was the 3rd thing that would leak?


so, here is the trigger

• IncenerationCast
  • Events
    • Unit - A unit Starts the effect of an ability
  • Conditions
    • (Ability being cast) Equal to Channel (Trigger Spell)
  • Actions
    • Quest - Display to (All players) the Quest Update message: SPELL HAS BEEN CAST
    • Set Temp_target1 = (Target unit of ability being cast)
    • Set Temp_Point1 = (Position of (Triggering unit))
    • Set Temp_Point2 = (Position of Temp_target1)
    • Unit - Create 1 Acid (Caster) for Player 1 (Red) at Temp_Point1 facing (Angle from Temp_Point1 to Temp_Point2) degrees
    • Set Temp_Caster2 = (Last created unit)
    • Unit - Order Temp_Caster2 to Neutral Alchemist - Acid Bomb Temp_target1
    • Unit - Create 1 Fire Trigger (Caster) for Player 1 (Red) at Temp_Point1 facing (Angle from Temp_Point1 to Temp_Point2) degrees
    • Set Temp_Caster1 = (Last created unit)
    • Unit - Order Temp_Caster1 to Night Elf Warden - Shadow Strike Temp_target1
    • Unit - Remove Temp_Caster2 from the game
    • Custom script: call RemoveLocation(udg_Temp_Point1)
    • Custom script: call RemoveLocation(udg_Temp_Point2)

 

[IcemanBo]

is (angle between 2 points) leaking?

That leaks 3 locations. Check the "Triggering tips" link in my signature for more information.

I meant if I saved the 2 points in a variable already, I will destroy these two points later. again, what was the 3rd thing that would leak?

It's correct that you don't leak if you store the both locations into variables and remove them after usage. A third location or something else isn't used that could leak, here's the code behind:

function AngleBetweenPoints takes location locA, location locB returns real
    return bj_RADTODEG * Atan2(GetLocationY(locB) - GetLocationY(locA), GetLocationX(locB) - GetLocationX(locA))
endfunction

or in general, does any arithmetic operation leak if they are not stored into a variable which then will be destroyed?

No, not really. It's just operations in the end, like 1+1, no object is created. For some background in general Memory Leaks.

 

[Map Designer]

It's correct that you don't leak if you store the both locations into variables and remove them after usage. A third location or something else isn't used that could leak, here's the code behind:

function AngleBetweenPoints takes location locA, location locB returns real
    return bj_RADTODEG * Atan2(GetLocationY(locB) - GetLocationY(locA), GetLocationX(locB) - GetLocationX(locA))
endfunction

No, not really. It's just operations in the end, like 1+1, no object is created. For some background in general Memory Leaks.

Does this mean my code is just fine? or is it better if I stored the angle in a variable Temp_angle? (I did update the trigger just now since I forgot to remove Caster2)

 

[IcemanBo]

It is just fine as it is.
Keep in mind, to make the calculation, you make a function call, and some maths operation. If you need this angle multiple times, then you might use a temp variable. It's basically also easier to read good variable names in code than reading all functions somewhere in between. But still, this has nothing todo with a leak.

 

[Map Designer]

It is just fine as it is.
Keep in mind, to make the calculation, you make a function call, and some maths operation. If you need this angle multiple times, then you might use a temp variable. It's basically also easier to read good variable names in code than reading all functions somewhere in between. But still, this has nothing todo with a leak.

Thanks!

the 3rd thing that would leak still bugs me though. what was that extra thing which would leak Had I not saved these variables?

 

[IcemanBo]

the 3rd thing that would leak still bugs me though.

As said there is no third leak. It was simply a mistake.

A third location or something else isn't used that could leak

Thanks!

Very welcome!

 

[loktar]

Uhm, quick question, do texttags (floating text) need to be destroyed when they are set to not be permanent (SetTextTagPermanent)?
The GUI trigger comment seems to imply that they're cleaned up automatically.

If so, I guess you need to use a sleepaction or timer to do it

 

[IcemanBo]

If they're removed via the lifespan if shouldn't leak.

 

[loktar]

Ah, great, thanks.

 

[Map Designer]

does pick every destructible function leak?

what about kill random destructible in region?

if it leaks, how do I make destructible group and destroy it?

 

[IcemanBo]

Enumerating destructables does not create something like a destructable group that could leak. You're safe here.

And as long you don't create a new region/rect for "Pick All Destructable in <Region>" there's no rect leak, too.

 

[Map Designer]

Enumerating destructables does not create something like a destructable group that could leak. You're safe here.

And as long you don't create a new region/rect for "Pick All Destructable in <Region>" there's no rect leak, too.

and if we pick all in region, then we have to destroy it dynamically?

 

[loktar]

The region should be destroyed unless it's one that you've created with the editor.

This is fine:

• Destructible - Pick every destructible in MyRegion <gen> and do (Destructible - Kill (Picked destructible))

This leaks a region:

• Destructible - Pick every destructible in (Region(0.00, 0.00, 0.00, 0.00)) and do (Destructible - Kill (Picked destructible))

Can be fixed like this:

• Custom script: set udg_TempRegion = Rect(0, 0, 0, 0)
• Destructible - Pick every destructible in TempRegion and do (Destructible - Kill (Picked destructible))
• Custom script: call RemoveRect(udg_TempRegion)
• Set TempRegion = No region

 

[Map Designer]

Hello

If I use dynamic destruction of Unit Group, Could it destroy my group at the end of each trigger? or does it destroy the group when it is assigned another value?

the first case can be a problem because it might destroy my group too early

 

[IcemanBo]

Link again on Memory Leaks.

 

[loktar]

local unit u

loop
    set u = FirstOfGroup(grp)
    exitwhen u == null
    call GroupRemoveUnit(grp, u)
endloop

Does the above code still leak a reference if we don't set u = null after the loop? I.e. is null still considered a reference if it's actually a null agent and not a plain null? Not sure if I explained that well.

 

[IcemanBo]

No it's no leak, null == null.
null is also only a part in memory, but a special one for default. One does not have any rights to write in this memory, one only can read it. So always when a function should return some object for example, but can't return any valid object for what ever reason (for example there is no FirstOfGroup() anymore), it can use the return null possibility (and should do). So just always when "null" is returned it does mean the function can't return valid values, and there's no further steps in difference of "null". In the end it's only important the agents point on some other memory not equal the object's memory it pointed to, and null is such other part in memory.

 

[loktar]

Great, thanks!

I asked because I think I saw somewhere (don't remember where) that a null string isn't necessarily the same as null when returning/concatenating (or something).
So I assume this just means that "" (empty string) is of type string, but null is not of type string?

 

[IcemanBo]

It's not very the same, yes.

When you have "" as string, it is a true string object. It has no chars in it, but you can do everything you want with it, like you can with any other strings. It can call functionality.
When you try to work with null, the string functionality can't be used. It's no real object, so concatenating (which is just a function in some way for the string object) isn't working.

But also some test cases:

case 1:
set s = null + "text" -> compiler error. (expected)

Next, this is more or less, too, as expected, case 2:

function nullAsString takes nothing returns string
    local string s // just for no inline
    return null
endfunction

local string s = nullAsString() + "test"

-> sums up to (null) , no valid string.

case 3 .. local string s = "test" + nullAsString() -> prints "test"
(at least from my theory above this is a bit unexpected)

And also, case 4:

local string s = null
set s = s + "test"

-> printes "test"
(might be unexpected like case 3, because null can not have functionality)

===

Both last cases would not work in normal c++, for example.

One might argue case3 works over case2 because the first string is actually always the string which actually calls the functionality, so the concatenate function, and so it is valid. First string is the calling object. And then there are internal checks for the second parameter, if the string to add is null, which will result in like adding an empty string/just nothing.

Case 4, there is maybe some implicit string conversion directly done here, when assining null to a string variable, local string s = null. So from then it's simply seen as empty string "".

===

So when having a conrete string variable, set to null, it may act as "", while working directly with null doesn't work with string concatenation.
Working with functions that return null (without being stored into a string variable) does not work for calling string concatenation as first parameter, but it will not cause an invalid string, when given as second parameter.

Basically as long as the string is only input for some function that jass vm controls, and is not the calling object itself like in string concatenation, null seems always be reacted same like empty string "". It's most likely some jass comfort that it is like this, checking for null at many places to keep nicer usage.