Garbage collection bug?

Started by
4 comments, last by Quittouff 12 years, 5 months ago
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
Advertisement
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.

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).

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.


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.

That's my bad, i forgot the destructor CFoo in my example making the release on the m_pObject.
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.
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.

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game


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.


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!

This topic is closed to new replies.

Advertisement