Sign in to follow this  
Quittouff

Garbage collection bug?

Recommended Posts

Hi again,

I have some crash in angelscript when I close my program. It seems that the garbage collector is doing something odd: deleting all functions table on every types and then ask for the GetRefCount (set to 0) on one of the types, but only under some specific circumstances. I spent hours trying to find out what is going on and narrowing it down to a very simple example so I could have a basic example that should allow you to reproduce the issue:

C++ code:
[source lang="cpp"]
class CFoo
{
public:
CFoo() : m_Ref(1) { }

void SetScriptObject(AngelScript::asIScriptObject* _pObject)
{
m_pObject = _pObject;
}

void AddRef()
{
m_Ref++;
}

void Release()
{
if( --m_Ref == 0 )
{
delete this;
}
}

static CFoo* CreateObject()
{
return new CFoo;
}


private:
AngelScript::asIScriptObject* m_pObject;
uint32 m_Ref;
};

class CDummy
{
public:
CDummy() {}

void AddRef() {}

void Release() {}
private:
uint32 m_DummyMember;
};
pEngine->RegisterInterface("IMyInterface");
pEngine->RegisterObjectType("CFoo", sizeof(CFoo), AngelScript::asOBJ_REF);
pEngine->RegisterObjectBehaviour("CFoo", AngelScript::asBEHAVE_ADDREF, "void f()", AngelScript::asMETHOD(CFoo, AddRef), AngelScript::asCALL_THISCALL);
pEngine->RegisterObjectBehaviour("CFoo", AngelScript::asBEHAVE_RELEASE, "void f()", AngelScript::asMETHOD(CFoo, Release), AngelScript::asCALL_THISCALL);
pEngine->RegisterObjectBehaviour("CFoo", AngelScript::asBEHAVE_FACTORY, "CFoo@ f()", AngelScript::asFUNCTION(&CFoo::CreateObject), AngelScript::asCALL_CDECL); pEngine->RegisterObjectMethod("CFoo", "void SetObject(IMyInterface@)", AngelScript::asMETHOD(CFoo, SetScriptObject), AngelScript::asCALL_THISCALL);

pEngine->RegisterObjectType("CDummy", sizeof(CDummy), AngelScript::asOBJ_REF);
pEngine->RegisterObjectBehaviour("CDummy", AngelScript::asBEHAVE_ADDREF, "void f()", AngelScript::asMETHOD(CFoo, AddRef), AngelScript::asCALL_THISCALL);
pEngine->RegisterObjectBehaviour("CDummy", AngelScript::asBEHAVE_RELEASE, "void f()", AngelScript::asMETHOD(CFoo, Release), AngelScript::asCALL_THISCALL);[/source]


Script code
[source lang="cpp"]
CBar test;
class CBase : IMyInterface
{
CDummy@ m_dummy; //Comment only this and everything is ok
}

class CBar : CBase
{
CBar()
{
m_foo.SetObject(this); //Comment only this and everything is ok
}
CFoo m_foo;
};
[/source]

Note: I'm still at the version 2.21.0 as I didn't see that it may be corrected in next versions

Share this post


Link to post
Share on other sites
Hi, my guess is that calling CFoo::SetScriptObject makes circular references that AS is unaware of (and crash happens because AS releases objects in some 'random' order without proper cleanup first).
Try implementing garbage collector behaviors for CFoo (as documented in Manual->Using AngelScript->Registering the app.. ->Advanced app... ->Garbage collected objects).
On the other note, [font=Consolas,]m_pObject [/font]is never released when set with CFoo::SetScriptObject. AS will AddRef any handles passed as parameters, but will not release them (until you register function with @+ autohandles), this is expected behavior for CFoo::SetScriptObject because you keep the reference but still [font=Consolas,]m_pObject[/font] needs to be released somewhere.

Share this post


Link to post
Share on other sites
[quote name='behc' timestamp='1318075330' post='4870465']
Hi, my guess is that calling CFoo::SetScriptObject makes circular references that AS is unaware of (and crash happens because AS releases objects in some 'random' order without proper cleanup first).
Try implementing garbage collector behaviors for CFoo (as documented in Manual->Using AngelScript->Registering the app.. ->Advanced app... ->Garbage collected objects).
[/quote]
Ho ok. So commenting the member in the base class would mess this random order and make it work properly, it makes sense. I'll try to implement the Garbage collected interface to see if it works better. Thanks.

[quote name='behc' timestamp='1318075330' post='4870465']
On the other note, [font="Consolas,"]m_pObject [/font]is never released when set with CFoo::SetScriptObject. AS will AddRef any handles passed as parameters, but will not release them (until you register function with @+ autohandles), this is expected behavior for CFoo::SetScriptObject because you keep the reference but still [font="Consolas,"]m_pObject[/font] needs to be released somewhere.
[/quote]
That's my bad, i forgot the destructor CFoo in my example making the release on the m_pObject.

Share this post


Link to post
Share on other sites
Adding gc behaviors to CFoo solved the problem on the example, and I integrated the solution in my actual classes with the same success.
Doing so I found an issue in the doc: EnumReferences and ReleaseAllReferences are with no parameter in the examples but they really take an asIScriptEngine* (which is much better as the object don't have to know directly the script engine), so I had some ESP errors at first :P.

Thanks behc for your precious help.

Share this post


Link to post
Share on other sites
Thanks for this report.

While the root cause was the lack of the GC behaviours, I've made some changes to make the library more robust in this scenario. Instead of crashing due to attempting to call a method that no longer exists, the engine will report an error message to the message stream and then simply skip the destruction the objects that couldn't be destroyed due to circular references with application objects. This will cause a memory leak, but at least it won't crash the application.

Share this post


Link to post
Share on other sites
[quote name='Andreas Jonsson' timestamp='1318440079' post='4871918']
Thanks for this report.

While the root cause was the lack of the GC behaviours, I've made some changes to make the library more robust in this scenario. Instead of crashing due to attempting to call a method that no longer exists, the engine will report an error message to the message stream and then simply skip the destruction the objects that couldn't be destroyed due to circular references with application objects. This will cause a memory leak, but at least it won't crash the application.
[/quote]

I just got the branch on SVN, this change is very usefull and save me a lot of time to see what classes causes some troubles and see if the game is still holding some references when it shouldn't! Thanks!

Share this post


Link to post
Share on other sites

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