[Visual77]

Hi. I've made a simple plugin that loops clients and checks their convar values. However, this plugin is causing a memory leak (datapack overflow) some time after plugin load. Plugin runs fine for maybe 25 minutes and then the leak starts. I've been trying to pinpoint where the exact problem is, but I haven't been able to find the cause.

I have a feeling that the leak starts if a QueryClientConVar fails to run, causing the created datapack not to free itself.

Can anyone take a look at the source and help me track down what the problem is?

I've Included a sm_dump_handles file where there's only 1 player on the server. There are over 1283 unclosed datapacks there. It doesn't grow constantly, but rather intermittently. So this means that it's not always leaking data.

[Mitchell]

Don't ignore the return value of QueryClientConVar, it's probably returning something and not exactly firing the callback (ClientQueryCallback_MatHack). Either that or queryclientconvar is taking too long to comeback or sometimes never comes back, you could make some kind of map which holds a timestamp to close the datapack after a certain time has expired.

[Visual77]

I've already tried checking the return value but it never logged a failure.

if (QueryClientConVar(client, cheatProtectedClientCvars[x][0], ClientQueryCallback_MatHack, data) == QUERYCOOKIE_FAILED)
{
	LogToPluginFile("Queried cvar %s on %N failed. Freeing data package.", cheatProtectedClientCvars[x][0], client);
	delete data;
}

or

if (!QueryClientConVar(client, cheatProtectedClientCvars[x][0], ClientQueryCallback_MatHack, data))
{
	LogToPluginFile("Queried cvar %s on %N failed. Freeing data package.", cheatProtectedClientCvars[x][0], client);
	delete data;
}

Originally Posted by Mitchell you could make some kind of map which holds a timestamp to close the datapack after a certain time has expired.

Might not be the best idea but maybe the only one possible. Do you mean something like sending the datapack to a timer and close it if it still exists?

[Mitchell]

https://sm.alliedmods.net/new-api/co...ryClientConVar
This does NOT return a boolean, adding a negative check to it could be the reason it isn't showing up.

[Visual77]

Return value returns 0 on failure and the cookie on sucssess. The cookie starts from 1 and each QueryClientConVar after = +1 to the cookie. I know this already, and have been checking for null return value, however as I said, the return value was always a positive value.

[ddhoward]

Originally Posted by Mitchell https://sm.alliedmods.net/new-api/co...ryClientConVar This does NOT return a boolean, adding a negative check to it could be the reason it isn't showing up.

Look at his first code snippet in post 3; he seems to be properly checking for equivalency to QUERYCOOKIE_FAILED, although the second should work too, since view_as<int>(QUERYCOOKIE_FAILED) == 0

[Fyren]

It might be there's some kind of edge case where the server doesn't provide anything back (like, perhaps the client disconnects immediately after the server sends a request) and so the callback where you close the handle doesn't happen. The timer idea would catch that.

[Visual77]

I don't work with datapacks normally (since CreateDataTimer usually suits my needs and is very automatic. This is all very basic, just testing.

CreateTimer(2.0, isDataStillValid, data);
QueryClientConVar(client, cheatProtectedClientCvars[x][0], ClientQueryCallback_MatHack, data);

public Action isDataStillValid(Handle timer, DataPack data)
{
	if (data != null)
	{
		delete data;
	}
}

results in runtime error inside timer isDataStillValid. I assume this is because the datapack was already destroyed in ClientQueryCallback_MatHack?

L 03/06/2018 - 11:58:43: [SM] Exception reported: Handle 5f170343 is invalid (error 3)
L 03/06/2018 - 11:58:43: [SM] Blaming: l4d2_clienthack_detect.smx
L 03/06/2018 - 11:58:43: [SM] Call stack trace:
L 03/06/2018 - 11:58:43: [SM]   [0] CloseHandle

[pride95]

maybe check:

PHP Code:


if(QueryClientConVar(client, cheatProtectedClientCvars[x][0], ClientQueryCallback_MatHack, data) == QUERYCOOKIE_FAILED)
{
   delete data;
} 


[Mitchell]

At the very least I would make an ArrayList for each client, and when ever a DataPack is created you add it to the list for the given client. Then you check the list within OnClientDisconnect to see if there are any valid DataPack handles still alive, close them and log them if they are. This would be easier then creating a Timer for each DataPack also. Also don't forget to search for the DataPack in the client's ArrayList first and delete it from the list before you delete the handle.

You'll need to use IsValidHandle(), as deleting (or closing) a handle wont change it's value to null else where.
https://sm.alliedmods.net/new-api/handles/IsValidHandle

Originally Posted by ddhoward Look at his first code snippet in post 3; he seems to be properly checking for equivalency to QUERYCOOKIE_FAILED, although the second should work too, since view_as<int>(QUERYCOOKIE_FAILED) == 0

Didn't see the full post of it checking the failed return value.