[cmake] Fix tests (/ static initialization order) #9146

fetzerch · 2016-02-15T23:55:10Z

This is an attempt to fix the unit tests when building with CMake.

79fe295 Fixes the static initialization order of CSpecialProtocol/RarManager:
The destructor of RarManager uses CSpecialProtocol (a class with only statics). Therefore we have to make sure that CSpecialProtocol is destructed after RarManager. According to the C++ standard, the order of con/destruction for statics is only guaranteed within the same compilation unit. Not a nice fix, I'm happy if anyone has a better idea.

Valgrind:

Invalid read of size 8
  in CSpecialProtocol::GetPath(std::string const&amp;) in /home/jcf/develop/kodi/kodi17/xbmc/filesystem/SpecialProtocol.cpp:268
  1: std::string::compare(std::string const&amp;) const in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.20
  2: bool std::operator&lt; &lt;char, std::char_traits&lt;char&gt;, std::allocator&lt;char&gt; &gt;(std::basic_string&lt;char, std::char_traits&lt;char&gt;, std::allocator&lt;char&gt; &gt; const&amp;, std::basic_string&lt;char, std::char_traits&lt;char&gt;, std::allocator&lt;char&gt; &gt; const&amp;) in /usr/include/c++/4.9/bits/basic_string.h:2590
  3: std::less&lt;std::string&gt;::operator()(std::string const&amp;, std::string const&amp;) const in /usr/include/c++/4.9/bits/stl_function.h:371
  4: std::_Rb_tree&lt;std::string, std::pair&lt;std::string const, std::string&gt;, std::_Select1st&lt;std::pair&lt;std::string const, std::string&gt; &gt;, std::less&lt;std::string&gt;, std::allocator&lt;std::pair&lt;std::string const, std::string&gt; &gt; &gt;::_M_lower_bound(std::_Rb_tree_node&lt;std::pair&lt;std::string const, std::string&gt; &gt;*, std::_Rb_tree_node&lt;std::pair&lt;std::string const, std::string&gt; &gt;*, std::string const&amp;) in /usr/include/c++/4.9/bits/stl_tree.h:1261
  5: std::_Rb_tree&lt;std::string, std::pair&lt;std::string const, std::string&gt;, std::_Select1st&lt;std::pair&lt;std::string const, std::string&gt; &gt;, std::less&lt;std::string&gt;, std::allocator&lt;std::pair&lt;std::string const, std::string&gt; &gt; &gt;::find(std::string const&amp;) in /usr/include/c++/4.9/bits/stl_tree.h:1913
  6: std::map&lt;std::string, std::string, std::less&lt;std::string&gt;, std::allocator&lt;std::pair&lt;std::string const, std::string&gt; &gt; &gt;::find(std::string const&amp;) in /usr/include/c++/4.9/bits/stl_map.h:860
  7: CSpecialProtocol::GetPath(std::string const&amp;) in /home/jcf/develop/kodi/kodi17/xbmc/filesystem/SpecialProtocol.cpp:268
  8: CSpecialProtocol::TranslatePath(CURL const&amp;) in /home/jcf/develop/kodi/kodi17/xbmc/filesystem/SpecialProtocol.cpp:166
  9: XFILE::CSpecialProtocolFile::TranslatePath(CURL const&amp;) in /home/jcf/develop/kodi/kodi17/xbmc/filesystem/SpecialProtocolFile.cpp:36
  10: XFILE::COverrideFile::Delete(CURL const&amp;) in /home/jcf/develop/kodi/kodi17/xbmc/filesystem/OverrideFile.cpp:61
  11: XFILE::CFile::Delete(CURL const&amp;) in /home/jcf/develop/kodi/kodi17/xbmc/filesystem/File.cpp:798
  12: XFILE::CFile::Delete(std::string const&amp;) in /home/jcf/develop/kodi/kodi17/xbmc/filesystem/File.cpp:785
  13: CRarManager::ClearCache(bool) in /home/jcf/develop/kodi/kodi17/xbmc/filesystem/RarManager.cpp:472
  14: CRarManager::~CRarManager() in /home/jcf/develop/kodi/kodi17/xbmc/filesystem/RarManager.cpp:67
  15: __run_exit_handlers in /build/buildd/glibc-2.21/stdlib/exit.c:82
  16: exit in /build/buildd/glibc-2.21/stdlib/exit.c:104
  17: (below main) in /build/buildd/glibc-2.21/csu/libc-start.c:323
Address 0x1d3d8bf0 is 32 bytes inside a block of size 48 free'd  1: operator delete(void*) in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so
  2: __gnu_cxx::new_allocator&lt;std::_Rb_tree_node&lt;std::pair&lt;std::string const, std::string&gt; &gt; &gt;::deallocate(std::_Rb_tree_node&lt;std::pair&lt;std::string const, std::string&gt; &gt;*, unsigned long) in /usr/include/c++/4.9/ext/new_allocator.h:110
  3: std::allocator_traits&lt;std::allocator&lt;std::_Rb_tree_node&lt;std::pair&lt;std::string const, std::string&gt; &gt; &gt; &gt;::deallocate(std::allocator&lt;std::_Rb_tree_node&lt;std::pair&lt;std::string const, std::string&gt; &gt; &gt;&amp;, std::_Rb_tree_node&lt;std::pair&lt;std::string const, std::string&gt; &gt;*, unsigned long) in /usr/include/c++/4.9/bits/alloc_traits.h:383
  4: std::_Rb_tree&lt;std::string, std::pair&lt;std::string const, std::string&gt;, std::_Select1st&lt;std::pair&lt;std::string const, std::string&gt; &gt;, std::less&lt;std::string&gt;, std::allocator&lt;std::pair&lt;std::string const, std::string&gt; &gt; &gt;::_M_put_node(std::_Rb_tree_node&lt;std::pair&lt;std::string const, std::string&gt; &gt;*) in /usr/include/c++/4.9/bits/stl_tree.h:389
  5: std::_Rb_tree&lt;std::string, std::pair&lt;std::string const, std::string&gt;, std::_Select1st&lt;std::pair&lt;std::string const, std::string&gt; &gt;, std::less&lt;std::string&gt;, std::allocator&lt;std::pair&lt;std::string const, std::string&gt; &gt; &gt;::_M_destroy_node(std::_Rb_tree_node&lt;std::pair&lt;std::string const, std::string&gt; &gt;*) in /usr/include/c++/4.9/bits/stl_tree.h:438
  6: std::_Rb_tree&lt;std::string, std::pair&lt;std::string const, std::string&gt;, std::_Select1st&lt;std::pair&lt;std::string const, std::string&gt; &gt;, std::less&lt;std::string&gt;, std::allocator&lt;std::pair&lt;std::string const, std::string&gt; &gt; &gt;::_M_erase(std::_Rb_tree_node&lt;std::pair&lt;std::string const, std::string&gt; &gt;*) in /usr/include/c++/4.9/bits/stl_tree.h:1247
  7: std::_Rb_tree&lt;std::string, std::pair&lt;std::string const, std::string&gt;, std::_Select1st&lt;std::pair&lt;std::string const, std::string&gt; &gt;, std::less&lt;std::string&gt;, std::allocator&lt;std::pair&lt;std::string const, std::string&gt; &gt; &gt;::~_Rb_tree() in /usr/include/c++/4.9/bits/stl_tree.h:715
  8: std::map&lt;std::string, std::string, std::less&lt;std::string&gt;, std::allocator&lt;std::pair&lt;std::string const, std::string&gt; &gt; &gt;::~map() in /usr/include/c++/4.9/bits/stl_map.h:96
  9: __run_exit_handlers in /build/buildd/glibc-2.21/stdlib/exit.c:82
  10: exit in /build/buildd/glibc-2.21/stdlib/exit.c:104
  11: (below main) in /build/buildd/glibc-2.21/csu/libc-start.c:323

We've seen already a couple of problems with the global initialization order. With this fix, the test execute is stable now at least on my local machine. Still it's not 100% sure that there are not more problems.

@notspiff, @afedchin ping.

akva2 · 2016-02-16T08:40:42Z

makes sense. i never saw this in my fork since rarmanager is gone.

xbmc/SystemGlobals.cpp

@@ -35,6 +35,10 @@
 #include "interfaces/python/XBPython.h"
 #endif

+// Guarantee that CSpecialProtocol is initialized before and uninitialized after RarManager
+#include "filesystem/SpecialProtocol.h"
+std::map<std::string, std::string> CSpecialProtocol::m_pathMap;


fetzerch · 2016-02-16T20:58:41Z

Seems like travis is still not happy. TestMassEvent.General and TestMultipleSharedSection.General still segfault (on clang3.5). Did any of you see them failing on your local machine? I've installed clang 3.5 now and still can't reproduce them. I executed the testsuite 100 times and it's been all stable for me :(

notspiff · 2016-02-16T21:10:57Z

I have seen em. they are extremely icky to track down, because they
typically are not deterministic. additionally you cannot cut down on the
number of tests executed because it's precisely interaction between tests
that takes place because the kodi-tests do not properly use fixtures that
cause the issues.

i know that
notspiff/kodi-cmake@e665b25
was added to quell some warnings while i was trying to track some of these
down, but they do not by themself fix issues. it was just to avoid some
noise. there used to be more fixes in there i'm not entirely sure where
they went, i might have dropped them in rebase rage at some point.
i believe my conclusion was that it was caused by compiler bugs. try
clang-3.6 i think you will find they are nowhere to be seen.

feb. 2016 21:59 skrev "Christian Fetzer" notifications@github.com:

Seems like travis is still not happy. TestMassEvent.General and
TestMultipleSharedSection.General still segfault (on clang3.5). Did any of
you see them failing on your local machine? I've installed clang 3.5 now
and still can't reproduce them. I executed the testsuite 100 times and it's
been all stable for me :(

—
Reply to this email directly or view it on GitHub
#9146 (comment).

fetzerch · 2016-02-18T21:00:07Z

@notspiff: Thanks, I tried a few more things, but I couldn't reproduce a crash even one single time. "Luckily" the tests mentioned above fail in almost 100% of the Travis runs, so I've added a commit that prints the stacktrace. Since you know the codebase much better than me, can you read anything meaningful out of it? A test with clang-3.6 didn't help btw, the issue remains.

https://travis-ci.org/xbmc/xbmc/builds/110214290:

[New LWP 28684]
[New LWP 28679]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `/home/travis/build/xbmc/xbmc/build/kodi-test --gtest_filter=TestMultipleSharedS'.

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x0000000003e21700 in ?? ()

Thread 2 (Thread 0x2ab2e881b940 (LWP 28679)):
#0  0x00002ab2f156a330 in std::string::_Rep::_M_destroy(std::allocator<char> const&) () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#1  0x0000000001079821 in CAdvancedSettings::~CAdvancedSettings() ()
#2  0x0000000001079ae9 in CAdvancedSettings::~CAdvancedSettings() ()
#3  0x00000000008b0f21 in xbmcutil::GlobalsSingleton<CAdvancedSettings>::Deleter<std::shared_ptr<CAdvancedSettings> >::~Deleter() ()
#4  0x00002ab2f1a06259 in __run_exit_handlers (status=0, listp=0x2ab2f1d886c8 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true) at exit.c:82
#5  0x00002ab2f1a062a5 in __GI_exit (status=<optimized out>) at exit.c:104
#6  0x00002ab2f19ebecc in __libc_start_main (main=0x855a50 <main>, argc=2, argv=0x7ffef5ebbcf8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7ffef5ebbce8) at libc-start.c:321
#7  0x0000000000855984 in _start ()

Thread 1 (Thread 0x2ab3006d4700 (LWP 28684)):
#0  0x0000000003e21700 in ?? ()
#1  0x0000000000e492a2 in XbmcCommons::ILogger::Log(int, char const*, ...) ()
#2  0x0000000000c507c2 in CThread::staticThread(void*) ()
#3  0x00002ab2e89cb182 in start_thread (arg=0x2ab3006d4700) at pthread_create.c:312
#4  0x00002ab2f1ac447d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111

I'll have a look also at your valgrind fixes.

The destructor of RarManager uses CSpecialProtocol (a class with only statics). Therefore we have to make sure that CSpecialProtocol is destructed after RarManager. The C++ standard guarantees the order of con/destruction for statics only within the same compilation unit.

It's useful to have asserts enabled when running the test suites.

This prevents an occasionally occurring crash. Program terminated with signal SIGSEGV, Segmentation fault. 0 0x0000000003e21700 in ?? () Thread 2 (Thread 0x2ab2e881b940 (LWP 28679)): 0 0x00002ab2f156a330 in std::string::_Rep::_M_destroy(std::allocator<char> const&) () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6 1 0x0000000001079821 in CAdvancedSettings::~CAdvancedSettings() () 2 0x0000000001079ae9 in CAdvancedSettings::~CAdvancedSettings() () 3 0x00000000008b0f21 in xbmcutil::GlobalsSingleton<CAdvancedSettings>::Deleter<std::shared_ptr<CAdvancedSettings> >::~Deleter() () 4 0x00002ab2f1a06259 in __run_exit_handlers (status=0, listp=0x2ab2f1d886c8 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true) at exit.c:82 5 0x00002ab2f1a062a5 in __GI_exit (status=<optimized out>) at exit.c:104 6 0x00002ab2f19ebecc in __libc_start_main (main=0x855a50 <main>, argc=2, argv=0x7ffef5ebbcf8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7ffef5ebbce8) at libc-start.c:321 7 0x0000000000855984 in _start () Thread 1 (Thread 0x2ab3006d4700 (LWP 28684)): 0 0x0000000003e21700 in ?? () 1 0x0000000000e492a2 in XbmcCommons::ILogger::Log(int, char const*, ...) () 2 0x0000000000c507c2 in CThread::staticThread(void*) () 3 0x00002ab2e89cb182 in start_thread (arg=0x2ab3006d4700) at pthread_create.c:312 4 0x00002ab2f1ac447d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111

fetzerch · 2016-02-20T09:42:18Z

jenkins build this please

[cmake] Fix tests (/ static initialization order)

fetzerch added the Type: Fix non-breaking change which fixes an issue label Feb 16, 2016

fetzerch changed the title ~~[cmake] Fix tests~~ [cmake] Fix tests (/ static initialization order) Feb 16, 2016

afedchin reviewed Feb 16, 2016
View reviewed changes

fetzerch force-pushed the cmake-fix-tests branch 5 times, most recently from c935aaf to f445eb8 Compare February 18, 2016 20:02

fetzerch force-pushed the cmake-fix-tests branch 3 times, most recently from 321c3a3 to e7bac52 Compare February 19, 2016 17:17

fetzerch added 4 commits February 20, 2016 09:04

[travis] Explicitly build with debug flags

21d1613

It's useful to have asserts enabled when running the test suites.

[travis] Print coredumps for failed tests

65aa269

fetzerch force-pushed the cmake-fix-tests branch from e7bac52 to 9f6aa55 Compare February 20, 2016 08:10

razzeee added Tests v17 Krypton labels Feb 20, 2016

fetzerch added a commit that referenced this pull request Feb 21, 2016

Merge pull request #9146 from fetzerch/cmake-fix-tests

41cbc6f

[cmake] Fix tests (/ static initialization order)

fetzerch merged commit 41cbc6f into xbmc:master Feb 21, 2016

fetzerch deleted the cmake-fix-tests branch February 21, 2016 07:58

fetzerch added this to the Krypton 17.0-alpha1 milestone Feb 21, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[cmake] Fix tests (/ static initialization order) #9146

[cmake] Fix tests (/ static initialization order) #9146

fetzerch commented Feb 15, 2016

akva2 commented Feb 16, 2016

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

fetzerch commented Feb 16, 2016

notspiff commented Feb 16, 2016

fetzerch commented Feb 18, 2016

fetzerch commented Feb 20, 2016

[cmake] Fix tests (/ static initialization order) #9146

[cmake] Fix tests (/ static initialization order) #9146

Conversation

fetzerch commented Feb 15, 2016

akva2 commented Feb 16, 2016

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

fetzerch commented Feb 16, 2016

notspiff commented Feb 16, 2016

fetzerch commented Feb 18, 2016

fetzerch commented Feb 20, 2016