Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Boost threads: in IOS, thread_info object is being destructed before the thread finishes executing

Our project uses a few boost 1.48 libraries on several platforms, including Windows, Mac, Android, and IOS. We are able to consistently get the IOS version of the project to crash (nontrivially but reliably) when using IOS, and from our investigation we see that ~thread_data_base is being called on the thread's thread_info while its thread is still running.

This seems to happen as a result of the smart pointer reaching a zero count, even though it is obviously still in scope in the thread_proxy function which creates it and runs the requested function in the thread. This seems to happen in various cases - the call stack is not identical between crashes, though there are a few variations which are common.

Just to be clear - this often requires running code which is creating hundreds of threads, though there are never more than about 30 running simultaneously. I have "been lucky" and got it very very early in the run also, but that's rare. I created a version of the destructor which actually catches the code red-handed:

in libs/thread/src/pthread/thread.cpp:

thread_data_base::~thread_data_base()
 {
   boost::detail::thread_data_base* const thread_info=detail::get_current_thread_data();
   void *void_thread_info = (void *) thread_info;
   void *void_this = (void *) this;
   // is somebody destructing the thread_data other than its own thread?
   // (remember that its own which should no longer point to it anyway,
   // because of the call to detail::set_current_thread_data(0) in thread_proxy)
   if (void_thread_info) { //  == void_this) {
     __builtin_trap();
   }
 }

I should note that (as seen from the commented-out code) I had previously checked to see that void_thread_info == void_this because I was only checking for the case where the thread's current thread_info was killing itself. I have also seen cases where the value returned by get_current_thread_data is non-zero and different from "this", which is really weird.

Also when I first wrote that version of the code, I wrote:

if (((void*)thread_info) == ((void*)this)) 

and at run-time I got some very weird exception that said I something about a virtual function table or something like that - I don't remember. I decided that it was trying to call "==" for this object type and was unhappy with that, so I rewrote as above, putting the conversions to void * as separate lines of code. That in itself is quite suspicious to me. I am not one to run to rush to blame compilers, but...

I should also note that when we did catch this happening the trap, we saw the destructor for ~shared_count appear twice consecutively on the stack in Xcode source. Very doubleweird. We tried to look at the disassembly, but couldn't make much out of it.

Again - it looks like this is always a result of the shared_count which seems to be owned by the shared_ptr which owns the thread_info reaching zero too early.

Update: it seems that it is possible to get into situations which reach the above trap without the situation doing any harm. Since fixing the issue (see answer) I have seen it happen, but always after thread_info->run() has finished executing. Don't yet understand how...but it's working.

Some additional info:

I should note that the boost.sh from Pete Goodliffe (and modified by others) that is commonly used to compile boost for IOS has the following note in the header:

: ${EXTRA_CPPFLAGS:="-DBOOST_AC_USE_PTHREADS -DBOOST_SP_USE_PTHREADS"}
# The EXTRA_CPPFLAGS definition works around a thread race issue in
# shared_ptr. I encountered this historically and have not verified that
# the fix is no longer required. Without using the posix thread primitives
# an invalid compare-and-swap ARM instruction (non-thread-safe) was used for the
# shared_ptr use count causing nasty and subtle bugs.
#
# Should perhaps also consider/use instead: -BOOST_SP_USE_PTHREADS

I use those flags, but to no avail.

I found the following which is very tantalizing - it looks like they had the same issue in std::thread:

http://llvm.org/bugs/show_bug.cgi?format=multiple&id=12730

That was suggestive of using an alternate implementation inside boost for arm processors which seems also to directly address this issue: spinlock_gcc_arm.hpp

The version included with boost 1.48 uses outdated arm assembly. I took the updated version from boost 1.52, but I'm having trouble compiling it. I get the following error: predicated instructions must be in IT block

I found a reference to what looks to be a similar use of this instruction here: https://zeromq.jira.com/browse/LIBZMQ-414

I was able to use the same idea to get the 1.52 code to compile by modifying the code as follows (I inserted an appropriate IT instruction)

__asm__ __volatile__(
 "ldrex %0, [%2]; \n"
 "cmp %0, %1; \n"
 "it ne; \n"
 "strexne %0, %1, [%2]; \n"
 BOOST_SP_ARM_BARRIER :
 "=&r"( r ): // outputs
 "r"( 1 ), "r"( &v_ ): // inputs
 "memory", "cc" );

But in any case, there are ifdefs in this file which look for the arm architecture, which is not defined that way in my environment. After I simply edited the file so that only ARM 7 code was left, the compiler complains about the definition of BOOST_SP_ARM_BARRIER:

In file included from ./boost/smart_ptr/detail/spinlock.hpp:35: ./boost/smart_ptr/detail/spinlock_gcc_arm.hpp:39:13: error: instruction requires a CPU feature not currently enabled BOOST_SP_ARM_BARRIER : ^ ./boost/smart_ptr/detail/spinlock_gcc_arm.hpp:13:32: note: expanded from macro 'BOOST_SP_ARM_BARRIER'

# define BOOST_SP_ARM_BARRIER "dmb"

Any ideas??

like image 486
Andy Weinstein Avatar asked Feb 04 '13 18:02

Andy Weinstein


1 Answers

Figured this out. It turns out that the boost.sh script that I mention in the question chose the incorrect boost flag to address this problem - instead of BOOST_SP_USE_PTHREADS (and the other flag there with it, BOOST_AC_USE_PTHREADS) it turns out that what is needed on IOS is BOOST_SP_USE_SPINLOCK. This ends up giving pretty much the identical solution used in the std::thread issue referred to in the question.

If you are compiling for any modern IOS device which uses ARM 7, but using an older boost (we are using 1.48), you need to copy the file spinlock_gcc_arm.hpp from a more recent boost (like 1.52). That file is #ifdef'd for the different arm architectures, but it is not clear to me that the defines it is looking for are defined in the IOS compile environment using the script. So you can either edit the file (violent but effective) or invest some time to figure out how to make this tidy and correct.

In any case, you may need to insert the extra assembly instruction that I did above in the question: "it ne; \n" I have not yet gone back to see if I can delete that now that I have my compile environment working problem.

However, we're not done yet. The code used in boost for this option includes, as discussed, ARM assembly language instructions. The ARM chips support two instruction sets which can't be mixed in a given module (not sure of the scope, but evidently file by file is an acceptable granularity when compiling). The instructions used in boost for this locking include non-Thumb instructions, but IOS by default uses the Thumb instruction set. The boost code, aware of the instruction set issue, checks to see that you have arm enabled but not thumb, but by default in IOS, thumb is on.

Getting the compiler to generate non-thumb ARM code depends on which compiler you are using in IOS - Apple's LLVM or LLVM GCC. GCC is deprecated, and Apple's LLVM is the default when you use XCode.

For the default Clang + Apple LLVM 4.1, you need to compile using the -mno-thumb flag. Also any files in your IOS app which use any part of boost which uses smart pointers will also have to be compiled using -mno-thumb.

To compile boost like this, I think you can just add -mno-thumb to the EXTRA_CPP_FLAGS in the script. (I modified the user-config.jam directly while experimenting and haven't yet gone back to clean up.)

For your app, in Xcode you need to select your target, then go into the Build Phases tab, and there select Compile sources. There you have the option of adding compile flags, so for each relevant file (which includes boost), add the -mno-thumb flag. You can do this directly in project.pbxproj also where each file has

settings = { COMPILER_FLAGS = ""; };   

you just change this to

settings = { COMPILER_FLAGS = "-mno-thumb"; }; 

But there's a little more. You also have to modify the darwin.jam file in the tools/build/v2/tools directory. In boost 1.48, there is a code that says:

    case arm :
    {
        options = -arch armv6;
    }

This has to be modified to

    case arm :
    {
        options = -arch armv7 ;
    }        

Finally, in the boost.sh script, in the function writeBjamUserConfig(), you should remove the references to -arch armv6.

If somebody knows how to do this a little more generally and cleanly, I'm sure we'd all benefit. For now, this is where I've gotten to, and I hope that this will help other IOS boost threads users. I hope that the various variants on the boost.sh IOS script out there will be updated. I plan to add some more links to this answer later.

Update: For a great article which describes the issue on the processor level,
see here: http://preshing.com/20121019/this-is-why-they-call-it-a-weakly-ordered-cpu

Enjoy!

like image 53
Andy Weinstein Avatar answered Nov 13 '22 14:11

Andy Weinstein