[Boost-users] [BUG]: Boost MP thread crasher: thread_cleanup key not being initialized

From: Bobby Thomale (rwthomale-ml_at_[hidden])
Date: 2003-10-20 17:10:01

I believe I have found a crashing bug in the boost thread classes on MacOS
X, in the MP threads implementation.

I looked at the source and I think I have figured out what is wrong and how
to fix it.  

The thread_cleanup function, which on the MP threads implementation is
called each time a boost thread exits, assumes that the "cleanup key" has
been initialized.  If it hasn't been initialized (ie you aren't using any
thread specific storage), it still calls MPGetTaskStorageValue with the
uninitialized key index.

Here are the first few lines of thread_cleanup:

void thread_cleanup()
{
    cleanup_handlers* handlers = reinterpret_cast<cleanup_handlers*>(
        MPGetTaskStorageValue(key));
    if(handlers != NULL)
    {
...

If you have not used any thread-specific storage, key is an uninitialized
variable when this gets called.  Not good.

It looks like for the most part, rather than puking, MPGetTaskStorageValue
is returning NULL when given an uninitialized key value, at least on 10.2.x.
(I am doing some testing on the upcoming 10.3 release of MacOS X and noted
that thread_cleanup crashes 100% of the time on this platform - I suspect
MPGetTaskStorageValue is no longer returning NULL.)  Since "key" is an
uninitialized variable, I would think this would have unpredictable results
and would crash occasionally even on 10.2.x, whenever "key" happened to
start off as a value that was already in use maybe.

It seems to me that we should simply make sure that the key is inited when
calling thread_cleanup like so:

boost::call_once(&init_cleanup_key, once);

This makes sure that the key is inited, even if get_handlers is never
called.  (Adding that line of code to the beginning of thread_cleanup
cleared up my crashing problems on 10.3 immediately.)

Is that proper use of the call_once mechanism?

Anyway, my first thought was to look at the implementations of threads on
the other platforms, since they don't have this call_once call in their
cleanup function before accessing the key either.

It looks to me like, at least in the Windows code, the cleanup function is
registered for the thread only during the get_handlers function, after the
key is inited.  If get_handlers is never called, the cleanup function never
gets registered.  

In the Mac implementation, however, cleanup_thread is called directly by the
thread proxy object at the end of thread execution, and it gets called even
if get_handlers has never been called and the key has never been
initialized.  

Here is a patch for my proposed fix.  (I am sure there are other ways of
fixing this too.)  

Comments, anyone?  

*** tss.orig.cpp    Mon Oct 20 13:19:44 2003
--- tss.cpp    Mon Oct 20 13:19:30 2003
***************
*** 126,131 ****
--- 126,133 ----
  
  void thread_cleanup()
  {
+     boost::call_once(&init_cleanup_key, once);
+     
      cleanup_handlers* handlers = reinterpret_cast<cleanup_handlers*>(
          MPGetTaskStorageValue(key));
      if(handlers != NULL)

                                -- Bobby

---------------------------------------------------------------------
Bobby Thomale
Senior Software Developer
Inoveon Corporation
http://www.inoveon.com/
---------------------------------------------------------------------