Re: [AD] Potential uthreads hang on cleanup?

[Peter Wang <tjaden@xxxxxxxxxx>]

Chris wrote:

> Peter Wang wrote:
>
> > The right way to do things is not to use pthread_cancel at all.  
> > Sorry, I was still new to threads when I wrote that code.  Patch 
> > attached, but I didn't test it with Kdevelop or whatever.
>
>
> Looks alright.. though I haven't tested it myself. Three things, though..
>
> > +static int thread_alive = FALSE;
>
>
> That should probably be volatile, shouldn't it? Since it gets changed 
> in a seperate thread you don't want the compiler making stupid 
> assumptions.


I've used this idiom without volatile elsewhere, and it seems to works 
anyway.  But adding volatile is probably a good idea.

> > +   ASSERT(thread_alive);
>
>
> I don't think that ASSERT is necesarry since AFAICT the only time the 
> thread is created is right after it's set to TRUE.


Well, ASSERTs are never "necessary".  They are there to state and 
enforce your assumptions.

> Plus, even if it is false, the thread will just close out on its own 
> anyway.


In that case, something's gone horribly wrong and I'd want to know about it.


Well, actually, the current ASSERT might trip up in this situation:

1. main thread sets thread_alive=TRUE and calls pthread_create
       ... however, the background thread doesn't get a chance to run ...
2. main thread immediately tries to shut down the thread:
       it sets thread_alive=FALSE and blocks on pthread_join
3. finally the background thread gets a chance to run:
       ASSERT(thread_alive) blows up

I really doubt it will happen in practice (and I don't know if it's 
possible with pthreads), but the BeOS port has some contortions to avoid 
this kind of situation[1].  So maybe I should remove the assertion on 
those grounds.

> > +   ASSERT(!!thread == !!thread_alive);
>
>
> Wouldn't ASSERT(thread && thread_alive); be faster? Or does the 
> compiler do some nice optimizations here?


It's short for ((!thread && !thread_alive) || (thread && thread_alive)).

Peter

[1] Initialise a semaphore with count=1, then create the thread and wait 
on the semaphore.  The first thing that the thread does when it runs is 
to decrement the semaphore count to indicate that it actually started.  
Unfortunately, we can't do the same thing using pthread mutexes since 
they are only meant to be unlocked by the thread that locked it (AFAIK).