Accepted...

Hi, 
 
As instructed, here is the summary about the pthread_join / 
pthread_detach issue. 
 
Thanks!   
 
Kevin 
 
  
Again based on comments from Matt Watson of Apple (Thank you Matt!), 
we 
seem to be doing a no-no in that we are using pthread_join to wait 
for a 
worker thread that has called pthread_detach on itself. 
 
From the pthread_join man page: 
 
       The  joined  thread  th  must be in the joinable state: it 
       must not have been detached using pthread_detach(3) or the 
       PTHREAD_CREATE_DETACHED attribute to pthread_create(3). 
 
 
If you look in store/source/stodmon.cxx you can see the 
That the OStorePageDaemon_Impl destructor (the OStorePageDaemon_Impl 
extends the osl::Thread class and defines its own run() method 
(similar to 
how the Java Thread class and run() method are typically used.) 
 
/* 
 * ~OStorePageDaemon_Impl. 
 */ 
OStorePageDaemon_Impl::~OStorePageDaemon_Impl (void) 
{ 
        if (isRunning()) 
        { 
                // Terminate. 
                terminate(); 
                m_aCond.set(); 
 
                // Join. 
                if (getCurrentIdentifier() != getIdentifier()) 
                        join(); 
        } 
} 
 
The destructor invokes terminate on other thread in "run" which is 
looping 
here: 
 
 /* 
 * run. 
 */ 
void SAL_CALL OStorePageDaemon_Impl::run (void) 
{ 
        setPriority (osl_Thread_PriorityBelowNormal); 
        while (schedule()) 
        { 
                TimeValue tmo; 
                tmo.Seconds = 10; 
                tmo.Nanosec = 0; 
 
                m_aCond.wait(&tmo); 
                flush(); 
                m_aCond.reset(); 
        } 
} 
 
 
osl_terminateThread then properly sets the flag for the "run" thread 
and 
 it exits the run() method and returns to the 
oslWorkerWrapperFunction 
 (see sal/osl/unx/thread.c) which in turn does a 
pthread_cleanup_pop(1) 
 which results in the oslCleanupFunction() being invoked which in 
turn 
 does a pthread_detach  on itself here: 
 
static void oslCleanupFunction(void* pData) 
{ 
        sal_Bool attached; 
 
        osl_TThreadImpl* pThreadImpl= (osl_TThreadImpl*)pData; 
 
        pthread_mutex_lock(&pThreadImpl->m_HandleLock); 
        pThreadImpl->m_Flags &= ~THREADIMPL_FLAGS_ACTIVE; 
 
        if ((attached = pThreadImpl->m_Flags & 
THREADIMPL_FLAGS_ATTACHED) 
 > 0) 
        { 
                pThreadImpl->m_Flags &= ~THREADIMPL_FLAGS_ATTACHED; 
 
                pthread_detach(pThreadImpl->m_hThread); 
        } 
... 
 
But the main thread (not the thread created to execute in the run() 
method 
is still in the destructor after terminate and is in a race to 
against the 
thread from "run()" exiting.  If the "run" thread exists first all 
is 
fine. 
 
If the thread in the OStorePageDaemon_Impl destructor gets to its 
join() 
 
              // Join. 
                if (getCurrentIdentifier() != getIdentifier()) 
                        join(); 
 
which invokes osl_joinWithThread()  first we end up doing a 
pthread_join 
 to wait for a thread that has run pthread_detach on itself. 
 
In fact my trace posted back on September 3rd shows exactly this: 
 
http://porting.openoffice.org/servlets/ReadMsg?msgId=396374&listName=dev 
 
Here is a snippet that shows what is going on. 
 
in OStorePageDaemon release - about to delete 0x4c2f00 
in ~OStorePageDaemon_Impl 
in ~OStorePageDaemon_Impl before join 
in ~OStorePageDaemon_Impl doing join 
OStorePageDaemon_Impl run returning 
OStorePageDaemon_Impl in on_Terminated 
right before pthread_cleanup_pop 
in oslCleanupFunction - about to detach 
in removeThreadID with initial pEntry 0x4c2df0 
in removeThreadID about to free 0x4c2df0 
this is the end my friends 
mach_port_deallocate(kernel_thread) failed: (os/kern) invalid name 
call th pthread_join returned 0 
 
Since we really want to do a "join" in the OStorePageDaemon_Impl 
 destructor we really should somehow not be allowing the thread to 
do the 
pthread_cleanup_pop(1) which invokes oslCleanupFunction and 
eventually 
pthread_detach. 
 
This is why commenting out pthread_detach in the oslCleanupFunction 
seems 
to work around the problem. 
 
I am not sure exactly how to tell sal that we want to "join" with a 
worker 
thread and not to do the pthread_detach which you defintely want to 
do if 
you don't care about the exit value or if the thread that would want 
to do 
the join (to wait) has somehow exited or terminated itself. 
 
I am not sure this causing the problem we are seeing under MacOSX 
10.2 but 
it is definitely a sequence of events that should not be happening 
given 
the pthread_join main page and my pthreads manual. 
 
Please let me know if you want me to file an issuezilla with this 
e-mail 
attached or let me know how you want to indicate that you want to 
wait on 
the worker thread and I can try to create a patch for sal that 
allows 
that. 
 
Thanks, 
 
Kevin 
 
 

With the exception that the originally suspected race condition turned
out to not exist (the 'child' thread signals it's startup to the
'parent' thread waiting on a condition; so access to the handle is
synchronized), careful code review (thanks Tino for helping) revealed
several other issues with this piece of code...

----------------------------
revision 1.23
date: 2002/10/30 21:05:08;  author: mhu;  state: Exp;  lines: +672 -682
#i8416# Fixed 'pthread_join()' / 'pthread_detach()' race condition,
several other race conditions, deadlock in 'osl_scheduleThread()' going
into suspend vs. 'osl_resumeThread()' using a redundant 2nd mutex,
static initialization not being thread safe, wrong 'spurious wakeup
protection' loops around 'pthread_cond_wait()' not checking a flag.
Added cleanup handlers to automatic cancellation points 'pthread_join()'
and 'pthread_cond_wait()'. Partial source cleanup.
----------------------------

As there was one race left, revision 1.24 contains the complete fix.

Fix confirmed on MacOSX, LinuxIntel and SolarisSparc, and none of the
other porting project complained -> resolved as fixed.

File 'porting/sal/osl/unx/thread.c' revision 1.24 needs to be tagged
for OOO_STABLE_1, OOO_STABLE_1_PORTS and OO643C.

Tagging confirmed (8861), closing...