Sign in to follow this  

Is this a memory leak ? (CScriptArray)

This topic is 2044 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi,

I create a CScriptArray in engine and return it to script like this

Function registration is this,
[CODE]
r = engine->RegisterObjectMethod("DataFile", "array<string>@ GetValuesOfChildrenWithName(const string &in, const string &in)", asMETHOD(DataFile, GetValuesOfChildrenWithName), asCALL_THISCALL);assert( r >= 0 );
[/CODE]

GetValuesOfChildrenWithName function in C++
(horrible function naming [img]http://public.gamedev.net//public/style_emoticons/default/smile.png[/img] it gets values of children nodes with the given name)
[CODE]
CScriptArray *DataFile::GetValuesOfChildrenWithName(const string &location, const string &name)
{
std::vector<string> allvalues;
ptree &tree = pt.get_child(location);
BOOST_FOREACH(boost::property_tree::ptree::value_type &v, tree)
{
if(v.first == name )
{
allvalues.push_back(v.second.get_value<string>());
}
}

asIObjectType* t = ScriptManager::getSingletonPtr()->engine->GetObjectTypeById(ScriptManager::getSingletonPtr()->engine->GetTypeIdByDecl("array<string>"));

CScriptArray* arr = new CScriptArray(allvalues.size(), t); // i do not Add Ref, since i dont need this array in C++
;

for (unsigned int i = 0; i< allvalues.size() ; ++i)
{
arr->InsertAt(i, new string(allvalues[i])); // this works, but would it cause a memory leak ?
}

return arr;
}
[/CODE]

Script function,
called about 600 times a second.
[CODE]
void Routine()
{
DataFile @df = DFM.GetDataFile("../../gamedata/newfile.xml");

array<string> arr = df.GetValuesOfChildrenWithName("Root", "SomeNode");

for( uint n = 0; n < arr.length; n++ )
{
//nothing happens here, for now.
}
}
[/CODE]

here is DFM.GetDataFile function, in case you suspect it
[CODE]
DataFile * DataFileManager::GetDataFile( const string &filename )
{
if(!dataFiles.count(filename))
{
return 0;
}

return &dataFiles[filename];
}
[/CODE]

DataFile is a asOBJ_REF | asOBJ_NOCOUNT type.

I don't have any leak detector programs at the moment, but program's memory usage is increasing at a rapid rate.

Is there a better way to send an array to script?
if not, What did i do wrong?

thank you. Edited by saejox

Share this post


Link to post
Share on other sites
Is this line how it's written in the script?
[CODE]array<string> arr = df.GetValuesOfChildrenWithName("Root", "SomeNode");[/CODE]

Without an @ on array<string>, you'll be copying the array each time the code is executed. If you aren't manually running a full cycle on the garbage collector, that could take a very long time to clean up.

Share this post


Link to post
Share on other sites
[quote name='ThyReaper' timestamp='1336425332' post='4938193']
Is this line how it's written in the script?
[CODE]array<string> arr = df.GetValuesOfChildrenWithName("Root", "SomeNode");[/CODE]

Without an @ on array<string>, you'll be copying the array each time the code is executed. If you aren't manually running a full cycle on the garbage collector, that could take a very long time to clean up.
[/quote]


[CODE]
array<string> arr = df.GetValuesOfChildrenWithName("Root", "SomeNode");
[/CODE]
and
[CODE]
array<string> @arr = df.GetValuesOfChildrenWithName("Root", "SomeNode");
[/CODE]

both yield the same result. memory keeps expanding.

i also call this every frame
[CODE]
engine->GarbageCollect(asGC_FULL_CYCLE);
[/CODE]
just to test if the garbage collector is slow to catch up.

i doesn't matter. memory keeps expanding. Edited by saejox

Share this post


Link to post
Share on other sites
[CODE]
CScriptArray* arr = new CScriptArray(6, t);

for (unsigned int i = 0; i< 6 ; ++i)
{
arr->InsertAt(i, new string(allvalues[i]));
}

[/CODE]

code above creates an array with 12 elements. i dont get why.
fixed it like this. (this is not the cause of the leak !)

[CODE]
CScriptArray* arr = new CScriptArray(0, t);
for (unsigned int i = 0; i< 6 ; ++i)
{
arr->InsertLast(new string(allvalues[i]));
}
[/CODE]

Anyway, memory leak still there.

Also
[url="http://www.angelcode.com/angelscript/sdk/docs/manual/doc_addon_array.html"]http://www.angelcode...ddon_array.html[/url]
example has a CScriptString. which doesnt exist anymore afaik.

i use scriptstdstring addon for std::string. Edited by saejox

Share this post


Link to post
Share on other sites
The memory leak is because you allocate a new string when passing the argument to the InsertAt/InsertLast. InsertAt/InsertLast will make a copy of the value, so you should be passing a pointer to the original value.

You're first example created 12 elements, because you created the array with the initial size of 6 elements, then inserted 6 more elements.

The correct implementation is:

[code]

// Create the array with the known length
CScriptArray* arr = new CScriptArray(6, t);
for (unsigned int i = 0; i< 6 ; ++i)
{
// Set the value of each element
arr->SetValue(i, &allvalues[i]);
}
[/code]

InsertAt or InsertLast would also work if used correctly, but it would be slower as the array would have to be resized with each new element added.

Thanks for pointing out the use of CScriptString in the manual. You're correct that the CScriptString is no longer an official add-on, so it shouldn't be used as the example. I'll fix this for the next update.

Regards,
Andreas

Share this post


Link to post
Share on other sites
Originally it was meant to be an internal method, but as it is now it is safe to be used from the application. I'll update it to make it a public method.

Thanks.

[edit] Done in revision 1300. Edited by Andreas Jonsson

Share this post


Link to post
Share on other sites

This topic is 2044 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this