Page MenuHomeWildfire Games

Fix potential GC-related crash in Profile
AbandonedPublic

Authored by wraitii on Nov 23 2020, 10:11 PM.

Details

Reviewers
None
Trac Tickets
#5859
Summary

Profile1 sometimes resets its state. In those situations, if there are outstanding "Stop" calls, it might end up that we delete the root, which crahes.
This happens semi-reliable in SM60 when starting a map (presumably because of GC needs). I think it can also happen under SM52, but haven't reproduced so far.

Since we are really interested in measuring slices, as they are what takes time, and those are guaranteed to return right away, we should simply profile that.

Test Plan

Run random games under SM60

Unit TestsFailed

TimeTest
0 msJenkins > cxxtest_debug.xml::[failed-to-read]
Failed to read test report file E:\Jenkins\workspace\vs2015-differential\cxxtest_debug.xml org.dom4j.DocumentException: Error on line 347 of document : Content is not allowed in trailing section. at org.dom4j.io.SAXReader.read(SAXReader.java:511)
0 msJenkins > TestAllocators::Debug Build & Tests / test_da
0 msJenkins > TestAllocators::Release Build & Tests / test_da
0 msJenkins > TestAllocators::test_da
0 msJenkins > TestAllocators::test_da
View Full Test Results (1 Failed · 1,699 Passed)

Event Timeline

wraitii created this revision.Nov 23 2020, 10:11 PM

Successful build - Chance fights ever on the side of the prudent.

builderr-debug-macos.txt
../../../source/simulation2/scripting/JSInterface_Simulation.cpp:154:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CFixedVector2D(-halfSize.X, -halfSize.Y),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
ld: warning: direct access in function 'RLMgCallback(mg_event, mg_connection*, mg_request_info const*)' from file 'obj/pyrogenesis_Debug/main.o' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'RLMgCallback(mg_event, mg_connection*, mg_request_info const*)' from file 'obj/pyrogenesis_Debug/main.o' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'tinygettext::POParser::parse()' from file '../../../binaries/system/libtinygettext_dbg.a(po_parser.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'JSI_Network::StartNetworkJoin(ScriptInterface::CmptPrivate*, CStrW const&, CStr8 const&, unsigned short, bool, CStr8 const&)' from file '../../../binaries/system/libengine_dbg.a(JSInterface_Network.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CStr8::Find(CStr8 const&) const' from file '../../../binaries/system/libengine_dbg.a(CStr.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CStr8::ReverseFind(CStr8 const&) const' from file '../../../binaries/system/libengine_dbg.a(CStr.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CStr8::AfterLast(CStr8 const&, unsigned long) const' from file '../../../binaries/system/libengine_dbg.a(CStr.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CStr8::BeforeLast(CStr8 const&, unsigned long) const' from file '../../../binaries/system/libengine_dbg.a(CStr.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CStr8::AfterFirst(CStr8 const&, unsigned long) const' from file '../../../binaries/system/libengine_dbg.a(CStr.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CStr8::BeforeFirst(CStr8 const&, unsigned long) const' from file '../../../binaries/system/libengine_dbg.a(CStr.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CStr8::Remove(CStr8 const&)' from file '../../../binaries/system/libengine_dbg.a(CStr.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CStr8::Replace(CStr8 const&, CStr8 const&)' from file '../../../binaries/system/libengine_dbg.a(CStr.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CTemplateLoader::LoadTemplateFile(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int)' from file '../../../binaries/system/libengine_dbg.a(TemplateLoader.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'SplitExts(char const*)' from file '../../../binaries/system/libengine_dbg.a(Util.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'SplitExts(char const*)' from file '../../../binaries/system/libengine_dbg.a(Util.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'COList::HandleAdditionalChildren(XMBElement const&, CXeromyces*)' from file '../../../binaries/system/libgui_dbg.a(COList.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
In file included from ../../../source/graphics/tests/test_Camera.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/graphics/tests/test_Camera.h:168:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CVector3D(-101.0f, -101.0f, 101.0f),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../../source/simulation2/tests/test_SerializeTemplates.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/simulation2/tests/test_SerializeTemplates.h:39:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        3, 0, 1, 4, 1, 5
                        ^~~~~~~~~~~~~~~~
                        {               }
1 warning generated.
ld: warning: direct access in function 'TestCLogger::ParseOutput()' from file 'obj/test_Debug/test_CLogger.o' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CxxTest::XmlFormatter::enterSuite(CxxTest::SuiteDescription const&)' from file 'obj/test_Debug/test_root.o' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'ts_str_contains(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)' from file 'obj/test_Debug/test_setup.o' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'tinygettext::POParser::parse()' from file '../../../binaries/system/libtinygettext_dbg.a(po_parser.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'JSI_Network::StartNetworkJoin(ScriptInterface::CmptPrivate*, CStrW const&, CStr8 const&, unsigned short, bool, CStr8 const&)' from file '../../../binaries/system/libengine_dbg.a(JSInterface_Network.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CStr8::Find(CStr8 const&) const' from file '../../../binaries/system/libengine_dbg.a(CStr.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CStr8::ReverseFind(CStr8 const&) const' from file '../../../binaries/system/libengine_dbg.a(CStr.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CStr8::AfterLast(CStr8 const&, unsigned long) const' from file '../../../binaries/system/libengine_dbg.a(CStr.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CStr8::BeforeLast(CStr8 const&, unsigned long) const' from file '../../../binaries/system/libengine_dbg.a(CStr.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CStr8::AfterFirst(CStr8 const&, unsigned long) const' from file '../../../binaries/system/libengine_dbg.a(CStr.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CStr8::BeforeFirst(CStr8 const&, unsigned long) const' from file '../../../binaries/system/libengine_dbg.a(CStr.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CStr8::Remove(CStr8 const&)' from file '../../../binaries/system/libengine_dbg.a(CStr.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CStr8::Replace(CStr8 const&, CStr8 const&)' from file '../../../binaries/system/libengine_dbg.a(CStr.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'CTemplateLoader::LoadTemplateFile(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int)' from file '../../../binaries/system/libengine_dbg.a(TemplateLoader.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'SplitExts(char const*)' from file '../../../binaries/system/libengine_dbg.a(Util.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'SplitExts(char const*)' from file '../../../binaries/system/libengine_dbg.a(Util.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'COList::HandleAdditionalChildren(XMBElement const&, CXeromyces*)' from file '../../../binaries/system/libgui_dbg.a(COList.o)' to global weak symbol 'std::__1::char_traits<char>::eq(char, char)' from file '../../../libraries/osx/gloox/lib/libgloox.a(clientbase.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
builderr-release-macos.txt
../../../source/simulation2/scripting/JSInterface_Simulation.cpp:154:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CFixedVector2D(-halfSize.X, -halfSize.Y),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libsimulation2.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libscriptinterface.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgraphics.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libatlas.a(precompiled.o) has no symbols
In file included from ../../../source/graphics/tests/test_Camera.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/graphics/tests/test_Camera.h:168:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CVector3D(-101.0f, -101.0f, 101.0f),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../../source/simulation2/tests/test_SerializeTemplates.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/simulation2/tests/test_SerializeTemplates.h:39:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        3, 0, 1, 4, 1, 5
                        ^~~~~~~~~~~~~~~~
                        {               }
1 warning generated.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/2096/display/redirect

wraitii requested review of this revision.Nov 23 2020, 11:13 PM

Had confirmation from a Spidermonkey dev that the incremental GC has no real obligation to finish soon, so the cycle start/end could well be frames apart, and thus definitely buggy wrt to the profiler.

The reason why this started being buggy in SM60 is that they changed the behaviour of the callback. The comment at the top comes from SM52 headers, whereas SM60 headers now say:

* During GC, the GC is bracketed by GC_CYCLE_BEGIN/END callbacks. Each
* slice between those (whether an incremental or the sole non-incremental
* slice) is bracketed by GC_SLICE_BEGIN/GC_SLICE_END.

Thus we wouldn't have had this issue in SM52. The fix of only considering SLICE is thus definitely correct, and can be committed as part of the SM60 API changes (but not for SM52, as I'd originally planned to do).

wraitii abandoned this revision.Nov 24 2020, 9:23 AM

Folded in D3116