Releasing Handles for global, dynamic arrays

[sparksterRK]

hello... would anyone know how to properly release handles when using global adt arrays? A plugin I use has many, and unloads frequently due to memory leak caused by these handles not closing properly. I've tried shrinking the block size when they are created, but it doesn't help, either. I'm just looking for a way to properly close or release the memory so the unloading stops.

An example of what i'm doing is:


new Handle:myArray;

public OnPluginStart() { myArray = CreateArray(3); }

[Dr. Greg House]

CloseHandle

[sparksterRK]

I've tried close handle. It just fires errors and things break, instead. I mean global because the arrays exist globally in the plugin; thru aren't contained within any specific function.

I assume if you do


new Handle:myArray[maxplayers + 1];

To close it you would not be able to just do:

CloseHandle(myArray);

Correct?


I don't have a lot of experience with this and there's not much to go on with plugins I've found that use adt arrays


I'm mobile so possible syntax mistakes.

[friagram]

No, your allocating an array of cells with nothing in it.
You would have to assign a handle to each cell in that array, and then close it if it's not invalid handle...

[sparksterRK]

I read from config files and fill the array later on. My question is really just how do you properly close them after they've been used, when a map ends, so that memory leaking doesn't occur.

[Chdata]

Like that? (found in adt_array.inc)

public OnMapEnd()
{
ClearArray(g_hMyArray);
}

[ddhoward]

I read from config files and fill the array later on. My question is really just how do you properly close them after they've been used, when a map ends, so that memory leaking doesn't occur.

Again, you just use CloseHandle.

Your problem is probably that you are not passing valid handles through CloseHandle, or are perhaps making the mistake you made in your second post.

new Handle:myArray[maxplayers + 1];
This line has created an ARRAY OF HANDLES. As in, there are MAXPLAYERS+1 handles represented here. Closing these multiple handles would require something like this:


for (new i = 0; i < sizeof(myArray); i++) {
if (myArray[i] != INVALID_HANDLE) {
CloseHandle(myArray[i]);
myArray[i] = INVALID_HANDLE;
}
}


What are the exact error messages you are getting? Also, posting some code would be nice, so we can show you what modifications you need to make.

[Chdata]

What's the difference between doing CloseHandle and ClearArray here?

[ddhoward]

What's the difference between doing CloseHandle and ClearArray here?

CloseHandle destroys the handle to the object. Once all of an object's handles are destroyed, the object itself is destroyed. (This is why CloneHandle() exists. Two handles, one cloned from the other, can be used to access the same object, such as the same cvar, or the same adt array. If CloseHandle() is called on only 1 of them, then the object will still survive, at least until the second handle is also closed. This is what allows two plugins to share the same array!)

ClearArray simply resizes the array to 0... but it's still there. It still exists, and it will still take up a little bit of memory. That, and your handle to the array object still exists, because you haven't closed that handle. I'm not sure how Sourcemod detects handle leaks for auto-unloading of leaky plugins, but if it's based in part on amount of existing handles... it will cause your plugin to be automatically unloaded after a sufficient amount of time.

[Powerlord]

You can check which plugins have which kinds of handles open using sm_dump_handles openhandles.txt which will dump the open handles to a file named openhandles.txt in the game's root steamcmd install directory (one above the game server's root).

Note that CVar handles are always owned by core and can't be closed.

CloseHandle destroys the handle to the object. Once all of an object's handles are destroyed, the object itself is destroyed. (This is why CloneHandle() exists. Two handles, one cloned from the other, can be used to access the same object, such as the same cvar, or the same adt array. If CloseHandle() is called on only 1 of them, then the object will still survive, at least until the second handle is also closed. This is what allows two plugins to share the same array!)

CloneHandle has one side effect: It makes the cloned Handle be owned by the plugin that called CloneHandle. This is important because, by default, only the plugin that called the function that created the Handle can call CloseHandle on it. (This is part of Handle Security (https://wiki.alliedmods.net/Handle_API_%28SourceMod%29#Handle_Security) on the SourceMod Core/Extension side)

[sparksterRK]

thanks for the help. i was trying to fix skys rpg plugin because he abandoned the project altogether (and i just read an email where i was told he closed the github account, too) and wasnt able to get any assistance from him.

in mapstart i do:

if (!b_MapStart) {

b_MapStart = true;

if (MainKeys == INVALID_HANDLE || !b_FirstLoad) MainKeys = CreateArray(16);


where firstload is always false and always true after the first mapstart, since i wasnt sure if they would default to INVALID_HANDLE or not.

on map end i do:


if (MainKeys != INVALID_HANDLE) {

CloseHandle(Handle:MainKeys);
MainKeys = INVALID_HANDLE;
}


and the same for the array of players using i <= MAXPLAYERS




there was some discussion earlier about arrays being declared without anything in them; this is true. what happens is the plugin reads multiple config (SMC_Parser) files, and then fills the array with them. because they can be edited to any size, dynamic arrays were needed (presumably?)


but the memory leaking and plugin unloading issue is fixed, so thanks a ton everyone!

[Chdata]

Here's a question.

Is it better to

A:

new g_hArray;

OnPluginStart()
{
g_hArray = CreateArray();
}

OnMapEnd()
{
ClearArray(g_hArray);
}

B:

new g_hArray;

OnMapStart()
{
g_hArray = CreateArray();
}

OnMapEnd()
{
CloseHandle(g_hArray);
}

I would guess A.

[Dr. Greg House]

You're doing two entirely different things there. And A will leak.

[Root_]

And A will leak.
Explain?

[WildCard65]

Here's a question.

Is it better to

A:

new g_hArray;

OnPluginStart()
{
g_hArray = CreateArray();
}

OnMapEnd()
{
ClearArray(g_hArray);
}

B:

new g_hArray;

OnMapStart()
{
g_hArray = CreateArray();
}

OnMapEnd()
{
CloseHandle(g_hArray);
}

I would guess A.

B doesn't leak handles, A does leak a handle. ClearArray clears the contents of the array but the handle is still valid. CloseHandle on the other hand frees the handle and calls the associated IHandleDispatch method for that handle.

Edit: This also answer's Root's question

[ddhoward]

The handle still exists in A. The array object (on the C++ side of sourcemod) still exists in A. Everything still exists in A, but the array size just happens to be 0.

The handle does not exist after B. The array object does not exist after B.

ClearArray() and CloseHandle() aren't two things you can compare, asking "which is better." They are two completely different things. It's like asking "Which is better, IsClientInGame() or GetClientEyeAngles() ?" They each have their own purpose.

[ddhoward]

Explain?

If you replace OnPluginStart() with OnMapStart() in A, then yes, every time the map changes a new handle is created, and a new array object is created. This array is resized to 0 when the map ends, but the object still exists, as well as the handle that points to it.

[Chdata]

But the thing is,

A. You only create the handle once, and you reuse it all the time. Why would this create a leak?
B. You recreate the handle every map change, and technically are "reusing it" all the time, so to speak you may be destroying an old one and making a new identical one.

Either way, shouldn't there only be 'one' handle for both methods that doesn't leak?

And with either method, you start with an empty array (though one is completely destroyed in B, and A just clears it).

That's what I'm comparing.

[ddhoward]

A. You only create the handle once, and you reuse it all the time. Why would this create a leak?
It would not, which is what I meant when I specified two posts ago that you must replace OnPluginStart() with OnMapStart() in order to cause a leak.
B. You recreate the handle every map change, and technically are "reusing it" all the time, so to speak you may be destroying an old one and making a new identical one.
You are not recreating a handle. The "new Handle:g_array;" instruction creates a SourcePawn variable that you can place a simple integer into. This integer is a POINTER to the actual array object. If you try printing to the chat the value of the variable, you will find that it is different each time the map starts.

[Chdata]

So about point B, you mean to say that CloseHandle does not close/destroy the handle that is pointed to by that int, and that doing g_hArray = CreateArray() again OnMapStart later does not create a new handle and return its pointer to be stored at g_hArray?

And about point A, so yeah, creating the array in OnPluginStart means it won't leak, in which case, is there anything wrong with doing that instead?

[comparing for the purposes of having a global dynamic array that starts out empty at the start of each map)

(sorry about asking my own off-topic question here, instead of making a new thread. it was just very similar anyway).

[BAILOPAN]

SourceMod internally allocates memory as reference-counted objects (that is, they count how many things reference the object, and when that reference drops to 0, the object is destroyed, freeing its memory).

A Handle represents one reference to an internal object, unless the Handle is 0 in which case it references nothing. Internal objects can be anything - resizeable arrays, menus, KeyValues, other plugins, etc.

CloseHandle() removes a reference to the object in the Handle, and then invalidates the Handle so it cannot be used anymore. If this causes the number of references on the object to drop to 0 (which is usually the case), the memory is freed.

CloneHandle() creates a new Handle referencing the same object as the original Handle, this increasing the number of references to the internal object by 1.

When a plugin is unloaded, all Handles it has created are closed, even if the plugin has forgotten them.

If you "forget" a Handle - as in, you lose its value without closing it, the underlying object will always have a reference of at least 1 until the plugin is unloaded. This can lead to memory leaking (until the plugin is unloaded).

[Powerlord]

SourceMod internally allocates memory as reference-counted objects (that is, they count how many things reference the object, and when that reference drops to 0, the object is destroyed, freeing its memory).

A Handle represents one reference to an internal object, unless the Handle is 0 in which case it references nothing. Internal objects can be anything - resizeable arrays, menus, KeyValues, other plugins, etc.

CloseHandle() removes a reference to the object in the Handle, and then invalidates the Handle so it cannot be used anymore. If this causes the number of references on the object to drop to 0 (which is usually the case), the memory is freed.

CloneHandle() creates a new Handle referencing the same object as the original Handle, this increasing the number of references to the internal object by 1.

When a plugin is unloaded, all Handles it has created are closed, even if the plugin has forgotten them.

If you "forget" a Handle - as in, you lose its value without closing it, the underlying object will always have a reference of at least 1 until the plugin is unloaded. This can lead to memory leaking (until the plugin is unloaded).

TL;DR, not closing a global Handle isn't a leak as long as you're not assigning a new value to said Handle's variable.

A:

new g_hArray;

OnPluginStart()
{
g_hArray = CreateArray();
}

OnMapEnd()
{
ClearArray(g_hArray);
}


You're doing two entirely different things there. And A will leak.

B doesn't leak handles, A does leak a handle. ClearArray clears the contents of the array but the handle is still valid. CloseHandle on the other hand frees the handle and calls the associated IHandleDispatch method for that handle.

i.e. you're both wrong, A isn't a Handle leak. The only error there is the missing tag on g_hArray in its declaration, which would be ArrayList using 1.7 syntax or Handle in 1.6 syntax

[sparksterRK]

While I do things a little bit differently than suggested, I've frequently checked the openhandles, and noticed that the plugin isn't leaking. However, I assume that in the SetConfigArrays function, it should be leaking, and don't really know why it isn't (as I don't CloseHandle the locally created arrays, after).

I also realize that the memory usage is definitely not optimized with the arrays created, and that there are a boat load of arrays. I'm curious that if you do CreateArray() without defining a block size, whether or not the block size will be auto-set based on the data being stored.

Someone mentioned that I was instantiating arrays with nothing in them. This is because config files are then parsed using smc_parser, and then the arrays are filled. Again, I'm not sure if this means that the block sizes can be set automatically, but it would be great, as I want to waste as little memory as possible.

It might answer questions, as well as create new ones.

[ddhoward]

I'm curious that if you do CreateArray() without defining a block size, whether or not the block size will be auto-set based on the data being stored.
If you do not set a block size, then a block size of 1 will be entered for you. The block size cannot be changed after array creation.

https://sm.alliedmods.net/api/index.php?fastload=show&id=685&