[JASS] How to remove leak properly?

[D.O.G.]

Very simple silly question inside:

    function Test takes nothing returns nothing
        local location p = Location(0.0, 0.0)
        /*
        do stuff here
        */
        call RemoveLocation(p)
        set p = null // The question - Do I need this line?
    endfunction

Same question for units and other handles.
I only know that it's unneccesary for integers, reals and booleans.

 

[deathismyfriend]

yes u need that line. u need to null all locals.

when using jass though u should not use locations.

 

[D.O.G.]

yes u need that line. u need to null all locals.

when using jass though u should not use locations.

Thanks! I don't use them) This is only example and location's init function is smaller than unit's)

SOLVED)

 

[PurgeandFire]

See the section "Local Variables" under this for a solid explanation of why you need to null locals:
http://wiki.thehelper.net/wc3/Variables

 

[edo494]

yes u need that line. u need to null all locals.

when using jass though u should not use locations.

a bit wrong, tou dont need to null reals etc, you cant in fact
you need to null all agents, because they are reference counted and if you dont free the reference(null it) it will keep the objects id in system just in case
this is very minor leak, but it has been done for years so its more or less required
correction: if what is said in that article is true(memory cant be freed) then it is very important to null locsl agents

 

[Imp Midna]

if what is said in that article is true(memory cant be freed) then it is very important to null locsl agents

Did anybody actually ever test this? While this surely is how agents in general work, it doesn't make sense to me in the context of warcraft where you actually have explicit destroy functions for each data type (DestroyGroup, RemoveLocation etc). Also, using destroyed variables often cause access violations, and it wouldn't make any sense if this memory was unaccessable but not freed after calling its deallocator.

 

[deathismyfriend]

u are right edo494. i shouldve said that u need to null all local agents. But yes wat that page says is true about the locals not being freed until u null them.

 

[PurgeandFire]

Did anybody actually ever test this? While this surely is how agents in general work, it doesn't make sense to me in the context of warcraft where you actually have explicit destroy functions for each data type (DestroyGroup, RemoveLocation etc). Also, using destroyed variables often cause access violations, and it wouldn't make any sense if this memory was unaccessable but not freed after calling its deallocator.

Yes, it is pretty easy to test with the task manager and a short period timer (e.g. 0.03 or 0.001). However, the statement edo mentioned is kind of iffy depending on the way you interpret it:

If an agent's reference count is not 0, its handle id and memory cannot be freed.

Destroying the handle itself frees memory -> the handle no longer exists. However, there is separate memory for the handle ID (and possibly other things...). Destroying the handle will free the memory associated with the handle, and nulling will ensure that the handle ID is recycled (as well as the memory associated with the handle ID). I assume that is what the quote's intent was. However, it can just as easily be read as the memory of the agent cannot be freed, which is not entirely true (it is partially true since part of the memory cannot be freed).

To test it, you run a func that has this for a periodic timer:

/* test for not nulling */
local location L = Location(0, 0)
call RemoveLocation(L)

/* test for removing and nulling */
local location L = Location(0, 0)
call RemoveLocation(L)
set L = null

/* test for not removing at all */
local location L = Location(0, 0)

And perhaps a test where you do nothing (as a control). You'll notice that the first test (removing but not nulling) has the memory in task manager climb up slowly. The second test (removing and nulling) will bounce around/stay relatively put memory-wise. The third test (no removing/nulling) will have the memory climb up relatively fast (faster than the first test).

This means that the destroy functions do affect the memory usage (which logically makes sense) even without nulling. However, nulling ensures that the other portion of memory is properly freed--otherwise there will be a leak (not as big as the leak from not destroying the handle, but it is still a leak).

Note that the article said this about agents. Other handles don't seem to suffer this issue. For example:

local texttag t = CreateTextTag()
call BJDebugMsg(I2S(GetHandleId(t)))
call DestroyTextTag(t)

If the issue existed with regular handle types, then this example would list a different number each time. However, it will show 99 each time. A simple periodic test in the task manager will show that the memory doesn't go up consistently either (test without the debug msg). However, we tend to null all locals anyway for good measure (a.k.a. too lazy to look up which ones need to be nulled/which ones don't).

 

[edo494]

well, when you check common.j there is:

//============================================================================
// Native types. All native functions take extended handle types when
// possible to help prevent passing bad values to native functions
//
type agent			    extends     handle  // all reference counted objects

I think they wont be lie in their "source" code

also

However, there is separate memory for the handle ID (and possibly other things...).

yea that makes sence, but I think they could make it bound to the object itself(shared_ptr in C++ for instance)

 

[Imp Midna]

Well, to my understanding, there are three main groups of memory that we can leak using an agent:

1: the handle id. This is a 4 byte integer that cannot be reused for other objects if its reference count never reaches 0. Instead, the id is gathering dust in its slot in the hashtable (or whatever data structure wc3 uses to save used handle ids, maybe they just have some "max id" counter and an array of a static size or smthn), taking its 4 bytes of space plus additional space for the container its sitting in (if they use one; again, if they have a static array, it doesnt waste any memory at all). Its therefor a very small leak and only important if you run out of handle ids in a huge map.
2: the associated handle-data. There is not much i can think that would belong in here (reference count, type of referenced object... that's it pretty much), but looking at the c++ library, I'm sure they found lots of ways to blow this data structure up to a unnecessarily huge size. I guess comparing it to a traditional std::whatever_ptr is the wrong thing to do as it can not possibly work that way, unless every handle id is mapped to a whatever_ptr pointing to the object, which would require wc3 to delete the whatever_ptr when nulling the handle in jass, which totally defeats the point of the whatever_ptr and... you get the idea. Its probably the kind of memory that is freed once the reference counter reaches 0 and therefor the memory we leak when we don't null our local variables (claps slowly and sarcastically in blizzards direction for making such a stupid error and then not fixing it)
3: The memory associated with the actual object that the handle references. This memory should be freed when calling its Destroy function, and we can assume that it is, as using a destroyed object causes the game to crash in most cases.

I guess my first impulse when hearing "the agents memory" was to think about 3, when in fact they mean 2. The problem is, however, i don't actually know if there isn't more memory, and using the task manager to observe the memory usage merely suggests that the process frees more memory when you null it, but unfortunately doesn't really clean up the question which part of the memory is freed by which method (maybe the Destroy function is just freeing some other data, and the majority or the important data is only really destroyed when the reference count reaches 0. The pure thought made me sort of paranoid.)

yea that makes sence, but I think they could make it bound to the object itself(shared_ptr in C++ for instance)

You mean entirely removing the destroy functions and purely relying on the reference count? thanks god they didn't do that

 

[edo494]

the funny thing is, pointer in whatever_ptr is freed when destructor is called
no it wouldnt require it to delete it, they for sure store everything in some array and when you null unit variable you dont free its handleId if the unit exists

and I meant that they could've made it like one object, class which would hold both int for handle id and the object itself