Re: [Boost-users] Re: Is threads CodeWarrior project broken?

From: Bobby Thomale (rwthomale-ml_at_[hidden])
Date: 2003-11-04 10:11:10

> It would be nice if the threads CW project could be fixed.

Yes - the project file is corrupted.  I think something must have happened
to it when it was checked into CVS - either it was interpreted as text
instead of binary, or something funky like that happened.  It is pretty easy
to recreate your own though - that is what I did and I am using the boost
threads successfully.

All you have to do is include all of the sources in the /libs/thread/src
folder and in the /libs/thread/src/mac folder.  It works just fine.

With the recent versions of Boost, however, some stuff broke in the thread
classes.  It's pretty easy to fix, I fixed it locally on my machine.  I need
to submit the changes back to get them into the CVS repository, I am just
lazy.  :-)  (Just kidding - I've been busy.)

Here's a description of the problem from my initial e-mail about it (edited
slightly for clarity):

> I am having trouble getting the MP implementation of threads to compile.  It
> seems to be having trouble with function pointers where the syntax
> function<type> is used.
> 
> I remember at one point it was mentioned that:
> 
> function<type, type...>
> 
> was deprecated and we were to use either the more universally supported:
> 
> functionN<type,type...>
> 
> or the cleaner: 
> 
> function<void myfuncSig()>
> 
> instead if our compiler supported it.
> 
> (Mine doesn't support that last one yet.)
> 
> I am guessing that the oldest syntax was removed and no one has bothered to
> update the Mac carbon thread implementaton to compile again since then?
> 
> That one seems pretty easy to fix - just update to a supported syntax.

^ I've done that, updated to the functionN<type,type> syntax and it fixed
the problem.  I need to submit back the changes but I haven't yet.
(SORRY!!!)  Just let the compiler lead you around by the nose if you'd like,
it will show you all of the places where this needs to change, and the
change is straightforward.  :-)

I've also discovered a crashing bug in the Carbon implementation.  This one
I submitted to the list a while ago, and saw no response.  It is a one-line
fix in the file tss.cpp; you can just copy and paste it in for now.

I probably just need to e-mail it directly to the guy who maintains the
thread code.  

Here's that e-mail:

> 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/
---------------------------------------------------------------------