Structs and leaks

07-21-2008, 07:44 PM

Okay, I just want to know if this is going to go leaking on me (I wasn't 100% sure and I wanted to be safe).

Here's an example.

JASS:


struct test
  group testgroup
  //more data
endstruct

function callback takes nothing returns nothing
  local test dat = //retrieve struct
  
  call DoThingsToGroup(dat.testgroup)
  call DoMoreStuff()

  call dat.destroy()
endfunction

function actions takes nothing returns nothing
  local group g = CreateGroup()
  local test t = test.create()  

  call GroupEnumUnitsInRange(....)
  set t.testgroup = g

  //Do stuff, like starting a periodic timer for the callback and setting the other members
  
call PolledWait(1000000000000.) //Not really that long. But it's to clarify THERE IS A WAIT HERE
  //After all of that:
  call DestroyGroup(g)
  call t.destroy()
  set g = null
endfunction

Will this leak a group? I destroy the originally made group (g) in the actions function, but for some reason something was telling me that "testgroup" would leak too, but I might just be crazy.

I know an onDestroy method would work, but if I need to keep this group (without the use of globals for whatever reason it may be), wouldn't destroying the struct (in the callback) fire the onDestroy method and remove that group, making the callback function useless? (since there's no group).

Pardon me if this is a stupid question/mistake or if I'm doing something wrong or if there's something I could be doing better. I'm still figuring out the minor things about structs (such as this.)

07-21-2008, 08:26 PM

tamisrah

First thing is you can't access your struct within the callback because you destroy the actual struct in your actions-function.

Same goes for the group when you destroy the group the reference 't.testgroup' points into the nirvana.

Therefor you have to do something like this:

JASS:

struct test
  group testgroup
  //more data
  
  method onDestroy takes nothing returns nothing
    call DestroyGroup(.testgroup)
    //============================================
    // set .testgroup = null
    // is afaik optional since it gets recycled 
    // as the struct id gets recycled
  endmethod
endstruct

function callback takes nothing returns nothing
  local test dat = //retrieve struct
  
  call DoThingsToGroup(dat.testgroup)
  call DoMoreStuff()

  if finished then
    call dat.destroy()
  endif
endfunction

function actions takes nothing returns nothing
  local group g = CreateGroup()
  local test t = test.create()  

  call GroupEnumUnitsInRange(....)
  set t.testgroup = g

  //Do stuff, like starting a periodic timer for the callback and setting the other members
  
  //After all of that:
  set g = null
endfunction

JASS:

struct test
  group testgroup
  //more data
  
  method onDestroy takes nothing returns nothing
    call DestroyGroup(.testgroup)
    //============================================
    // set .testgroup = null
    // is afaik optional since it gets recycled 
    // as the struct id gets recycled
  endmethod
endstruct

function callback takes nothing returns nothing
  local test dat = //retrieve struct
  
  call DoThingsToGroup(dat.testgroup)
  call DoMoreStuff()

  if finished then
    call dat.destroy()
  endif
endfunction

function actions takes nothing returns nothing
  local group g = CreateGroup()
  local test t = test.create()  

  call GroupEnumUnitsInRange(....)
  set t.testgroup = CreateGroup()
  call GroupAddGroup(t.testgroup,g)

  //Do stuff, like starting a periodic timer for the callback and setting the other members
  
  //After all of that:
  call DestroyGroup(g)
  set g = null
endfunction

To answer your answer more precisely:
1. No, you wouldn't leak a group but you can't use it in the callback either.
2. I'm not exactly certain what you want to do there, but I think you think right there.
You indeed have to make the destruction of the struct within the callback dependend on
some condition (the if finished in my code) to avoid that problem you (probably) describe.

07-21-2008, 08:34 PM

darkwulfv

I thought it'd be understood there'd be a wait between the callback and the deletion =.= Sorry about that.

Anyways... I'll add in a wait (and get attacked for it) for clarification.

And wouldn't I have to destroy the struct I made in the actions function as well? (Since it's creating one?) I'm almost positive I do. (I could have sworn that when you make a local struct you destroy it when you're done with that instance of it.)

Here's an actual example (one where I'm actually using it) to help specify why I'm confused.

JASS:

struct PC_Data
  unit caster
  group targets
endstruct
  
private function Callback takes nothing returns nothing
  local timer t = GetExpiredTimer()
  local PC_Data dat = GetHandleInt(t, "CPdata")
  local unit u = dat.caster
  local group g = CreateGroup()
  local unit f 
  
  call GroupAddGroup(dat.targets, g)
  set f = FirstOfGroup(g)
  
  call UnitDamageTarget(u, f, ArmorDamage(f, DAMAGE + (GetUnitAbilityLevel(u, ABILITY_ID) * 2), TYPE), false, false, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNIVERSAL, null)
  call DestroyEffect(AddSpecialEffectTarget(TARGET_SFX, f, "chest"))
  call GroupRemoveUnit(g, f)
  call GroupRemoveUnit(dat.targets, f)
  set dat.targets = g
  
  call dat.destroy()
  set t = null
  set u = null
  set f = null
endfunction

See how the group is used constantly? (It is set at the struct's creation in the action function) If I were to destroy the struct (with an onDestroy method that destroyed the group), I'd lose the group. Which is why I'm wondering if setting a struct member to something requires it to be destroyed properly, if just destroying the original value will take care of it.

07-21-2008, 11:18 PM	#4
Anitarf	Umm, dude... what's your function supposed to do, exactly? Is Callaback being called from a periodic or one-shot timer? Show us the rest of the code please. What's it supposed to do anyway? You're doing a lot of really weird stuff in there.

07-21-2008, 11:33 PM

chobibo

You won't leak a group as long as you keep a reference to it if you want to destroy it later.

example:

struct Test
    group testgroup
endstruct

function actions takes nothing returns nothing
    local Test data=Test.create()
    local group g=CreateGroup()
    // Set g to data.testgroup
    set data.testgroup=g
    // Do stuff to group within the struct
    call data.destroy()
    // Destroy yhe struct
    call FirstOfGroup(g)
    // Do stuff to group g
    call DestroyGroup(g)
    set g=null
endfunction

07-21-2008, 11:34 PM

darkwulfv

I'm sorry, I'll clarify (rest of code isn't really needed, but if my explanation still doesn't work I'll post it.).

The callback function is called by a periodic timer. It takes the stored group and gets the first unit, then damages it and adds an SFX, and then removes it from the group. The periodic timer fires at a rate equal to "1 second divided by # of targets" (so it will always go through all of them in one second). Once all the units have been damaged, it groups them again and repeats. (It's a channeling spell.)

My question arises because if I destroy that struct and it has an onDestroy method (to clean the group handle) , the group will be removed and the spell will break after the first unit. So, if I destroy the original group handle that got put into the struct (in the actions function), do I have to worry about any leaking?

Hmm... I'm wondering why I made a local group, added the stored group to it, then removed the first unit from both of them. I think it was because I was unsure whether you could modify stored members directly like that. (I told you I don't know all the details about structs yet =p)

I'm sorry for continuous confusion; I've been somewhat rushed and I keep forgetting to add important details (like what a function does). I hope I explained it well enough to get my question through. But to be sure, he's the question as summed up as I can make it.

If I take a group, put it in a struct, then use the struct in a periodic callback function but do NOT destroy the group w/ the struct (for whatever reason), but I DO destroy the original group handle (from the function it was set it in), will that leak?

EDIT: I think Chobibo might have answered my question, but I'm not sure. That's similar to what I do, but he isn't using the struct in a seperate callback function like I do.

EDITEDIT: Quickie roughsketch of what I do:

JASS:

struct blah
  group blah
endstruct

function callback takes nothing returns nothing
  local struct blah = GetStruct()
  
  call DoStuffToGroup
  
  call blah.destroy()
//Notice that the group is not destroyed because there is no onDestroy method
endfunction

function actions takes nothing returns nothing
  local group g = CreateGroup()
  local timer t = CreateTimer()
  local struct blah = blah.create

call FillGroupWithUnits(g, ...)

  set blah.group = g

call TimerStart(...) //Yes it's periodic
call AttachStructToTimer()

call PolledWait(10.)

call DestroyGroup(g)
call PauseTimer(t)
call DestroyTimer(t)
call blah.destroy()

set g = null
set t = null

Does THAT leak?

07-21-2008, 11:51 PM

Anitarf

First of all, you use the struct to attach data (in this case, a unit and a group) to a timer. It only makes sense to destroy such a struct when you destroy the timer, in other words, destroying the struct in the callback of a periodic timer makes no sense; the periodic timer will continue to run the function, so the struct needs to continue to exist.

Furthermore, your actions with the group (or rather, groups) make no sense. You're creating a new group and in the end, you end up storing it in the struct, thus loosing the reference to the original group which you then can't properly destroy. Couldn't you just remove units from the original group?

How about something like this?

JASS:

private function Callback takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local PC_Data dat = GetHandleInt(t, "CPdata")
    local unit f = FirstOfGroup(dat.targets)
  
    if f == null then
        //if the group isn't destroyed in the struct's destroy method (recommended) then destroy it here
        call dat.destroy()
        call DestroyTimer(t) //probably a better idea would be to recycle timers
    else
        call UnitDamageTarget(dat.caster, f, ArmorDamage(f, DAMAGE + (GetUnitAbilityLevel(dat.caster, ABILITY_ID) * 2), TYPE), false, false, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNIVERSAL, null)
        call DestroyEffect(AddSpecialEffectTarget(TARGET_SFX, f, "chest"))
        call GroupRemoveUnit(dat.targets, f)
    endif
    set f = null
    set t = null
endfunction

07-22-2008, 12:05 AM

darkwulfv

Quote:

Really? I've been told to do it differently (I've seen it done differently as well). Either way, the code still works, whether I destroy the struct created in the callback or not. I've never had a problem with it when I do that.

Quote:

Furthermore, your actions with the group (or rather, groups) make no sense. You're creating a new group and in the end, you end up storing it in the struct, thus loosing the reference to the original group which you then can't properly destroy. Couldn't you just remove units from the original group?

Already realized that.

Quote:

Originally Posted by Me

Hmm... I'm wondering why I made a local group, added the stored group to it, then removed the first unit from both of them. I think it was because I was unsure whether you could modify stored members directly like that. (I told you I don't know all the details about structs yet =p)

Your method actually makes things quite a bit easier for a couple spells. I still swear I read (or was told) somewhere to destroy the struct instance you make in the callback, and like I said, i was doing that before without an issue.
But with your method I can safely use some onDestroy methods without worrying about losing reference to important members.

I guess that wraps things up. Thanks for the help, +rep and all that.

Oh, and regarding:

Quote:

//probably a better idea would be to recycle timers

I think I understand what you mean (isn't there a system for that?), but I don't know what system I should be using. Doesn't CSSafety do that? (but doesn't that require the entire Caster System, which I don't need?)

07-22-2008, 12:08 AM

chobibo

Quote:

If I take a group, put it in a struct, then use the struct in a periodic callback function but do NOT destroy the group w/ the struct (for whatever reason), but I DO destroy the original group handle (from the function it was set it in), will that leak?

The handle won't leak since you keep a reference to it and then destroy it later.

double free:

struct blah
  group blah
endstruct

function callback takes nothing returns nothing
  local struct blah = GetStruct()
  
  call DoStuffToGroup
  
  call blah.destroy()
//Notice that the group is not destroyed because there is no onDestroy method
endfunction

function actions takes nothing returns nothing
  local group g = CreateGroup()
  local timer t = CreateTimer()
  local struct blah = blah.create

call FillGroupWithUnits(g, ...)

  set blah.group = g

call TimerStart(...) //Yes it's periodic
call AttachStructToTimer()

call PolledWait(10.)

call DestroyGroup(g)
call PauseTimer(t)
call DestroyTimer(t)
//call blah.destroy() // double free of struct blah, you don't need to do this since you already unreferenced it in the callback.

set g = null
set t = null

07-22-2008, 12:29 AM

#10

Anitarf

Quote:

Originally Posted by darkwulfv

You were probably told that for one-shot timers.

The reason it still worked for you is because you didn't have a chaotic enough testing environment. The data is still there after you destroy the struct (unless you destroy it in the onDestroy method; which is a good reason to do so, because when doing so becomes inconvenient you get a good indicator that you're using structs wrong), but the struct's index is up for grabs when the game needs to create a new struct of that type, so if another spell is cast and a new struct is created recycling that index, the data will be lost (and leaked in case of handles such as groups) as new data overwrites it (not to mention how badly things fall apart once you have two instances of a spell working with the same data, and then one of the spell instances ends and cleans up that data).

Quote:

As far as I know CSSafety can be used outside the caster system (it only requires CSData if you use the most recent version).

07-22-2008, 03:05 AM

#11

darkwulfv

Quote:

The reason it still worked for you is because you didn't have a chaotic enough testing environment. The data is still there after you destroy the struct (unless you destroy it in the onDestroy method; which is a good reason to do so, because when doing so becomes inconvenient you get a good indicator that you're using structs wrong), but the struct's index is up for grabs when the game needs to create a new struct of that type, so if another spell is cast and a new struct is created recycling that index, the data will be lost (and leaked in case of handles such as groups) as new data overwrites it (not to mention how badly things fall apart once you have two instances of a spell working with the same data, and then one of the spell instances ends and cleans up that data).

Ahhh, I understand now. Spells using structs weren't being cast nearly enough or overlapping to cause issues like that.

Quote:

The data is still there after you destroy the struct (unless you destroy it in the onDestroy method; which is a good reason to do so, because when doing so becomes inconvenient you get a good indicator that you're using structs wrong),

That was exactly what made me create this thread.

Okay, I think I understand all this. So, I only need destroy the struct at the end of the spell, and with an onDestroy it'll clean everything up? Sweet. Thanks again for the help.

Quote:

As far as I know CSSafety can be used outside the caster system (it only requires CSData if you use the most recent version).

orly? I'll check it out, thanks.

07-22-2008, 06:00 PM

#12

Anitarf

Quote:

Originally Posted by darkwulfv

Okay, I think I understand all this. So, I only need destroy the struct at the end of the spell, and with an onDestroy it'll clean everything up? Sweet. Thanks again for the help.

That's over-simplifying things. You destroy the struct when it's no longer needed, in this case when you destroy the timer, which happens at the end of one damage loop, not at the end of the spell, which according to your description is made out of multiple consecutive damage loops.

07-22-2008, 08:40 PM	#13
darkwulfv	Ohh, alright. I think I understand now.