/tmp/cvs610a0a90.1

3 files changed, 90 insertions, 42 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 89ea5b3af..2b86cf186 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -5,6 +5,23 @@
 
 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.
 	* pthread.cc: Change __pthread_self to pthread::self() thruoghout.
 	(_pthread_cleanup_push): New function.
diff --git a/winsup/cygwin/thread.cc b/winsup/cygwin/thread.cc
index dbcfc5a9e..cc24fdca6 100644
--- a/winsup/cygwin/thread.cc
+++ b/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)
@@ -1547,12 +1559,16 @@ __pthread_exit (void *value_ptr)
   thread->pop_all_cleanup_handlers();
 
   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->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;
+      return EDEADLK;
     }
 
-  else if( __pthread_equal(thread, &joiner ) )
+  (*thread)->mutex.Lock ();
+
+  if((*thread)->attr.joinable == PTHREAD_CREATE_DETACHED)
     {
       if (return_val)
         *return_val = NULL;
-      return EDEADLK;
+      (*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;
     }
 
-  // force cleanup on exit
-  (*thread)->joiner = *thread;
-  (*thread)->attr.joinable = PTHREAD_CREATE_DETACHED;
+  // 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;
 }
diff --git a/winsup/cygwin/thread.h b/winsup/cygwin/thread.h
index 25f20b482..6c28abcbb 100644
--- a/winsup/cygwin/thread.h
+++ b/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);