Opened 8 years ago
Last modified 7 years ago
#4154 closed defect
Crash - All runtimes must be destroyed before calling JS_ShutDown — at Version 5
Reported by: | elexis | Owned by: | |
---|---|---|---|
Priority: | Must Have | Milestone: | Alpha 23 |
Component: | Core engine | Keywords: | |
Cc: | Patch: |
Description (last modified by )
When I start the random map script "Syria" and quit the game while the loading screen is about 20%, chances are high that it runs into a breakpoint:
ScriptEngine.h(43): Assertion failed: "m_Runtimes.size() == 0 && "All runtimes must be destroyed before calling JS_ShutDown!"" Assertion failed: "m_Runtimes.size() == 0 && "All runtimes must be destroyed before calling JS_ShutDown!""
Could it be related to r18584?
Change History (5)
comment:1 by , 8 years ago
comment:2 by , 8 years ago
I've figured out where the problem most likely is.
In CMapGeneratorWorker::RunThread, we create a ScriptInterface and delete it again when this function has completed. So we must ensure that it completes before we destroy the parent runtime in the main thread. That should be ensured by the destructor of CMapGeneratorWorker because it joins the threads using pthread_join (so the worker thread terminates before the main thread does).
However, this destructor does not get called because there is a memory leak in the case when loading is aborted by closing the window. The lifetime of ~CMapGeneratorWorker is tied to CMapReader (~CMapReader > ~CMapGenerator > CMapGeneratorWorker). And CMapReader leaks because it's deleted at the end of the loading process inside CMapReader::DelayLoadFinished, which never happens in this case.
So we need to properly delete the CMapReader object when loading is cancelled.
EDIT: Actually there are two additional leaks in ps/LoaderThunks.h (the two "new" statements) if the registered functions are never called.
comment:3 by , 8 years ago
Priority: | Should Have → Must Have |
---|
comment:4 by , 7 years ago
Milestone: | Backlog → Alpha 22 |
---|---|
Priority: | Must Have → Release Blocker |
Summary: | All runtimes must be destroyed before calling JS_ShutDown → Crash - All runtimes must be destroyed before calling JS_ShutDown |
Putting this into the list of tickets few people look at rarely. Doesn't have to stay.
comment:5 by , 7 years ago
Description: | modified (diff) |
---|---|
Milestone: | Alpha 22 → Backlog |
Priority: | Release Blocker → Must Have |
r18584 added that assertion and made the problem appear, but I guess it was there before. The proper order is to call JS_Init first, then create one or more runtimes, use them, destroy all of them and then call JS_Shutdown. We have added this assertion to ensure that no script runtimes are remaining when JS_Shutdown gets called. Apparently there still are active script runtimes in this particular case (or our test is broken).
I'll have a look.