addExternClass / removeExternClass from within SystemLogic

[ataylor]

I am hunting down and fixing memory leaks in our Unigine 2.2.1 application.

 

There were leaks in some of our standalone classes after calling Unigine::Interpreter::addExternClass in constructors but failing to call Unigine::Interpreter::removeExternClass in destructors. I added in the necessary removeExternClass calls and this resolved the leaks.

 

However, in the case of Unigine::SystemLogic it seems different. We have our own MySystemLogic which derives from Unigine::SystemLogic, currently with the same memory leak problems from calling addExternClass without matching removeExternClass. If I add in calls to removeExternClass in ~MySystemLogic, I get a crash on app close with the following call stack:

 

Unigine::ExternClassExport::clear() Line 1812 C++

Interpreter::clear() Line 7677 C++

EngineInterpreter::clear() Line 317 C++

World::clear() Line 364 C++

Engine::shutdown() Line 1469 C++

Engine::~Engine() Line 542 C++

 

I have tried calling addExternClass / removeExternClass in MySystemLogic::init and MySystemLogic::shutdown instead of constructor / destructor, but still there is the same crash on app close. How can I resolve this?

[silent]

Hi Adam,

 

Is it possible to have a small scene with memory leaks and crash at shutdown?

Thanks!

[ataylor]

Hi silent.

 

It looks like this is not actually specific to SystemLogic, just specific to our particular case. It seems that the extern classes that our SystemLogic is trying to remove still have existing instances in UnigineScript environment. 

 

In other words, calling removeExternClass on a class with active instances in the UnigineScript environment causes a crash in Engine shutdown. Of course there shouldn't be any instances remaining which is our problem, but still I would hope engine would handle this more gracefully.

 

I haven't made reproduction scene yet, but maybe with the above info you will be able to reproduce the problem.

[ataylor]

I've done some investigation and have more info about this.

 

Here is some basic structure of our application:

class MyExternClass()
{
...
}

MySystemLogic::MySystemLogic()
{
Unigine::Interpreter::addExternClass("MyExternClass", Unigine::MakeExternClass<MyExternClass>();
}

MySystemLogic::~MySystemLogic()
{
Unigine::Interpreter::removeExternClass("MyExternClass");
}

int main(int argc, char** argv)
{
MyApp app();
Unigine::EnginePtr engine(UNIGINE_VERSION, &app, argc, argv);
MySystemLogic systemLogic();
MyWorldLogic worldLogic();
engine->main(&systemLogic, &worldLogic, NULL);
worldLogic.shutdown();
systemLogic.shutdown();
app.shutdown();
return 0;
}

It seems that something in our UnigineScript environment is holding references to instances of MyExternClass (I posted question in separate thread about this here https://developer.unigine.com/forum/topic/4072-track-object-references-in-uniginescript/) which aren't released until main function exits, the EnginePtr is deleted/shutdown due to going out of scope, which calls World::clear, which calls Interpreter::clear, which attempts to call destructors for remaining classes. However, by this point, MySystemLogic has already been destroyed and the extern class has been removed, so there is a crash when trying to call the extern class destructor.

 

So, it seems I need to identify why there are still instances of MyExternClass alive, or I need to restructure things so Engine can be deleted before MySystemLogic.

[shodan]

Hi there,

 

I've been able to repeat the issue, and looks like it *should* be possible to work it around by simply shuffling the code around a bit, one way or another.

 

int main(int argc, char** argv)
{
MyApp app();
Unigine::EnginePtr engine(UNIGINE_VERSION, &app, argc, argv);
MySystemLogic systemLogic();
MyWorldLogic worldLogic();
engine->main(&systemLogic, &worldLogic, NULL);
worldLogic.shutdown();
systemLogic.shutdown();
app.shutdown();
return 0;
}

 

First, you aren't really supposed to call shutdown() explicitly. It is supposed to be automatically called by the engine, at the appropriate time, during engine.shutdown() essentially.

 

Second, I did a quick test, and it does not actually get called for me. Looks like a bug on our side. I will investigate more.

 

Third, engine.shutdown() is called implicitly here, via the destructor, and so are the logics destructors. So they are called backwards, your logics get destroyed first, and class binding gets removed in the destructor, engine get destroyed next.

 

However, engine still needs those class bindings during the shutdown => logics should be destroyed after the engine => moving the declaration of systemLogic up, so it happens before the Unigine::EnginePtr engine(...) line, should help with that part.

 

Fourth, if (if) there are members that point to engine Nodes (or other objects) in MySystemLogic, then auto-destroying those will start to crash after that. Because the engine is gone, and all pointers are gone with it. Now, ideally you would want to kill off such objects in shutdown()... but unless I screwed up with my quick test, it does not get called, which is another issue. And a workaround for that would be to perform a separate manual_shutdown() call anywhere before the return 0.

 

Overall, it's all about the call sequence in the end. To avoid crashes, you have to removeExternClass() after the engine is gone, not before. Currently, you're implicitly doing this before (via the destructors).

 

Note that moving the remove...() to shutdown() would *not* help even if the call was working: systemLogic.shutdown() gets called early in the shutdown process, interpreter shutdown happens later.

 

Hope that helps!

[shodan]

Second, I did a quick test, and it does not actually get called for me. Looks like a bug on our side. I will investigate more.

 

Aww. Ignore that. My mistake, it does get called actually. (I must have confused the scopes or made a typo or something...)

 

If you call engine.shutdown() explicitly, immediately after engine->main(), then the shutdown() methods in logics classes *do* get called alright. Therefore, I suggest this:

 

1. Add an explicit engine.shutdown() call, immediately after engine->main(). Just for clarity if nothing else. Note the dot, not the arrow. The dot is important.

 

2. Place all the code that releases any pointers to engine things (Nodes, Objects, whatever) in the respective MyLogic::shutdown() calls. It will be called during engine->shutdown() then.

 

That fixes everything on my test. And no variable reordering is require this way.

[ataylor]

Reviving this old thread after a while working on other projects.

 

This application has been updated to use Unigine 2.4. I see the Interpreter handles leftover externs on engine shutdown much more gracefully now, which is great. This is an improvement, but I would still like to do things correctly.  :)

 

Thanks shodan, the overall application structure and ideal order of operations is much clearer now.

 

I am still wondering about one core thing -- what are the recommended places to call Interpreter::addExtern* and removeExtern* functions inside MyWorldLogic and MySystemLogic classes? Constructor / destructor results in externs left over when the engine is destroyed before the logics. Putting them in init / shutdown results in interpreter parser errors about unknown tokens in between world loads. My solution right now is to make it asymmetrical -- calling addExtern* in logic constructors but manually calling removeExtern* immediately before I know engine will go out of scope and be destroyed. This works (mostly) but feels pretty awkward.