/tmp/cvs610a0a90.1

winsup/cygwin/ChangeLog

@@ -3,6 +3,23 @@
 	* fhandler_disk_file.cc (fhandler_disk_file::fstat_helper): Correctly
 	set number of links for directory, if appropriate.
 
+2002-06-10  Robert Collins  <rbtcollins@hotmail.com>
+
+	* thread.cc: Variation of a patch from Thomas Pffaf.
+       	(__pthread_detach): Cleanup thread object if the thread has terminated.
+	(__pthread_join): Change order of error checks, and lock against
+	join/detach/exit races.
+	(__pthread_exit): Lock object against join/detach/exit races.
+	(pthread::thread_init_wrapper): Ditto.
+	(thread_init_wrapper): Rename to pthread::thread_init_wrapper.
+	(pthread::create): Check that the mutex initialized correctly.
+	(pthread::push_cleanup_handler): Lock against potential cancellation
+	race. NB: this may not be required if pthread_cleanup_push is non-
+	cancelable.
+	* thread.h (pthread::mutex): New member.
+	(thread_init_wrapper): Rename to pthread::thread_init_wrapper.
+	(pthread::thread_init_wrapper_: New static member.
+
 2002-06-10  Robert Collins  <rbtcollins@hotmail.com>
 
 	* cygwin.din: Add _pthread_cleanup_push and _pthread_cleanup_pop.

winsup/cygwin/thread.cc

@@ -384,6 +384,14 @@ pthread::create (void *(*func) (void *), pthread_attr *newattr,
   function = func;
   arg = threadarg;
 
+  if (verifyable_object_isvalid (&mutex, PTHREAD_MUTEX_MAGIC) != VALID_OBJECT)
+    {
+      thread_printf ("New thread object access mutex is not valid. this %p",
+		     this);
+      magic = 0;
+      return;
+    }
+
   win32_obj_id = ::CreateThread (&sec_none_nih, attr.stacksize,
 				(LPTHREAD_START_ROUTINE) thread_init_wrapper,
 				this, CREATE_SUSPENDED, &thread_id);
@@ -409,8 +417,10 @@ pthread::push_cleanup_handler (__pthread_cleanup_handler *handler)
   if (this != self ())
     // TODO: do it?
     api_fatal ("Attempt to push a cleanup handler across threads"); 
+  mutex.Lock();
   handler->next = cleanup_handlers;
   cleanup_handlers = handler;
+  mutex.UnLock();
 }
 
 void
@@ -923,7 +933,7 @@ verifyable_object_isvalid (void const * objectptr, long magic)
 
 /* Pthreads */
 void *
-thread_init_wrapper (void *_arg)
+pthread::thread_init_wrapper (void *_arg)
 {
   // Setup the local/global storage of this thread
 
@@ -955,9 +965,11 @@ thread_init_wrapper (void *_arg)
   /*the OS doesn't check this for <= 64 Tls entries (pre win2k) */
   TlsSetValue (MT_INTERFACE->thread_self_dwTlsIndex, thread);
 
+  thread->mutex.Lock();
   // if thread is detached force cleanup on exit
   if (thread->attr.joinable == PTHREAD_CREATE_DETACHED && thread->joiner == NULL)
     thread->joiner = pthread::self ();
+  thread->mutex.UnLock();
 
 #ifdef _CYG_THREAD_FAILSAFE
   if (_REENT == _impure_ptr)
@@ -1548,11 +1560,15 @@ __pthread_exit (void *value_ptr)
 
   MT_INTERFACE->destructors.IterateNull ();
   
+  thread->mutex.Lock();
   // cleanup if thread is in detached state and not joined
   if( __pthread_equal(&thread->joiner, &thread ) )
     delete thread;
   else
+    {  
       thread->return_ptr = value_ptr;
+      thread->mutex.UnLock();
+    }
 
   if (InterlockedDecrement (&MT_INTERFACE->threadcount) == 0)
     exit (0);
@@ -1569,24 +1585,27 @@ __pthread_join (pthread_t *thread, void **return_val)
   if (verifyable_object_isvalid (thread, PTHREAD_MAGIC) != VALID_OBJECT)
     return ESRCH;
 
-  if ((*thread)->attr.joinable == PTHREAD_CREATE_DETACHED)
+  if ( joiner == *thread)    
-    {
-      if (return_val)
-        *return_val = NULL;
-      return EINVAL;
-    }
-
-  else if( __pthread_equal(thread, &joiner ) )
     {
       if (return_val)
         *return_val = NULL;
       return EDEADLK;
     }
 
+  (*thread)->mutex.Lock ();
+
+  if((*thread)->attr.joinable == PTHREAD_CREATE_DETACHED)
+    {
+      if (return_val)
+        *return_val = NULL;
+      (*thread)->mutex.UnLock ();
+      return EINVAL;
+    }
   else
     {
       (*thread)->joiner = joiner;
       (*thread)->attr.joinable = PTHREAD_CREATE_DETACHED;
+      (*thread)->mutex.UnLock ();
       WaitForSingleObject ((*thread)->win32_obj_id, INFINITE);
       if (return_val)
          *return_val = (*thread)->return_ptr;
@@ -1605,14 +1624,24 @@ __pthread_detach (pthread_t *thread)
   if (verifyable_object_isvalid (thread, PTHREAD_MAGIC) != VALID_OBJECT)
     return ESRCH;
 
+  (*thread)->mutex.Lock ();
   if ((*thread)->attr.joinable == PTHREAD_CREATE_DETACHED)
     {
+      (*thread)->mutex.UnLock ();
       return EINVAL;
     }
 
+  // check if thread is still alive
+  if (WAIT_TIMEOUT == WaitForSingleObject ((*thread)->win32_obj_id, 0) )
+    {
       // force cleanup on exit
       (*thread)->joiner = *thread;
       (*thread)->attr.joinable = PTHREAD_CREATE_DETACHED;
+      (*thread)->mutex.UnLock ();
+    }
+  else
+    // thread has already terminated.
+      delete (*thread);
 
   return 0;
 }

winsup/cygwin/thread.h

@@ -229,6 +229,34 @@ public:
    ~pthread_attr ();
 };
 
+class pthread_mutexattr:public verifyable_object
+{
+public:
+  int pshared;
+  int mutextype;
+    pthread_mutexattr ();
+   ~pthread_mutexattr ();
+};
+
+class pthread_mutex:public verifyable_object
+{
+public:
+  CRITICAL_SECTION criticalsection;
+  HANDLE win32_obj_id;
+  LONG condwaits;
+  int pshared;
+  class pthread_mutex * next;
+
+  int Lock ();
+  int TryLock ();
+  int UnLock ();
+  void fixup_after_fork ();
+
+  pthread_mutex (pthread_mutexattr * = NULL);
+  pthread_mutex (pthread_mutex_t *, pthread_mutexattr *);
+  ~pthread_mutex ();
+};
+
 class pthread:public verifyable_object
 {
 public:
@@ -264,44 +292,20 @@ public:
    void pop_cleanup_handler (int const execute);
 
    static pthread* self ();
+   static void *thread_init_wrapper (void *);
 
 private:
     DWORD thread_id;
     __pthread_cleanup_handler *cleanup_handlers;
+    pthread_mutex mutex;
 
     friend void __pthread_exit (void *value_ptr);
+    friend int __pthread_join (pthread_t * thread, void **return_val);
+    friend int __pthread_detach (pthread_t * thread);
+    
     void pop_all_cleanup_handlers (void);
 };
 
-class pthread_mutexattr:public verifyable_object
-{
-public:
-  int pshared;
-  int mutextype;
-    pthread_mutexattr ();
-   ~pthread_mutexattr ();
-};
-
-class pthread_mutex:public verifyable_object
-{
-public:
-  CRITICAL_SECTION criticalsection;
-  HANDLE win32_obj_id;
-  LONG condwaits;
-  int pshared;
-  class pthread_mutex * next;
-
-  int Lock ();
-  int TryLock ();
-  int UnLock ();
-  void fixup_after_fork ();
-
-  pthread_mutex (unsigned short);
-  pthread_mutex (pthread_mutexattr *);
-  pthread_mutex (pthread_mutex_t *, pthread_mutexattr *);
-  ~pthread_mutex ();
-};
-
 class pthread_condattr:public verifyable_object
 {
 public:
@@ -410,8 +414,6 @@ int __pthread_detach (pthread_t * thread);
 
 extern "C"
 {
-void *thread_init_wrapper (void *);
-
 /*  ThreadCreation */
 int __pthread_create (pthread_t * thread, const pthread_attr_t * attr,
 		      void *(*start_routine) (void *), void *arg);