2011-05-17 Ivan Maidanski <ivmai@mail.ru>

	* pthread_support.c (pthread_join): Add assertion (check thread is
	finished).
	* pthread_support.c (GC_register_my_thread): Don't detach the
	thread if invoked from the thread destructor.
	* win32_threads.c (GC_register_my_thread): Ditto.
	* win32_threads.c (GC_unregister_my_thread): Don't delete the
	thread (just set FINISHED) if the thread is not detached (only if
	GC_PTHREADS); add assertion (check the thread is not finished).
	* tests/threadkey_test.c (main): Join some created threads.

diff --git a/ChangeLog b/ChangeLog
index e7cd2c7..bb57cc5 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,17 @@
 2011-05-17  Ivan Maidanski  <ivmai@mail.ru>
 
+       * pthread_support.c (pthread_join): Add assertion (check thread is
+       finished).
+       * pthread_support.c (GC_register_my_thread): Don't detach the
+       thread if invoked from the thread destructor.
+       * win32_threads.c (GC_register_my_thread): Ditto.
+       * win32_threads.c (GC_unregister_my_thread): Don't delete the
+       thread (just set FINISHED) if the thread is not detached (only if
+       GC_PTHREADS); add assertion (check the thread is not finished).
+       * tests/threadkey_test.c (main): Join some created threads.
+
+2011-05-17  Ivan Maidanski  <ivmai@mail.ru>
+
        * pthread_support.c (GC_delete_gc_thread): Rename "gc_id" local
        variable to "t".
        * win32_threads.c (GC_delete_gc_thread): Ditto.

diff --git a/pthread_support.c b/pthread_support.c
index 58d515d..f2d1754 100644 (file)
--- a/pthread_support.c
+++ b/pthread_support.c
@@ -1250,6 +1250,7 @@ GC_API int WRAP_FUNC(pthread_join)(pthread_t thread, void **retval)
     if (result == 0) {
         LOCK();
         /* Here the pthread thread id may have been recycled. */
+        GC_ASSERT((t -> flags & FINISHED) != 0);
         GC_delete_gc_thread(t);
         UNLOCK();
     }
@@ -1402,20 +1403,8 @@ GC_API int GC_CALL GC_register_my_thread(const struct GC_stack_base *sb)
 
     LOCK();
     me = GC_lookup_thread(self);
-    if (0 == me || (me -> flags & FINISHED) != 0) {
-        if (me == 0) {
-          me = GC_register_my_thread_inner(sb, self);
-        } else {
-          /* This code is executed when a thread is registered from the */
-          /* client thread key destructor.                              */
-          GC_record_stack_base(me, sb);
-          me -> flags &= ~FINISHED;
-#         ifdef GC_EXPLICIT_SIGNALS_UNBLOCK
-            /* Since this could be executed from a thread destructor,   */
-            /* our signals might be blocked.                            */
-            GC_unblock_gc_signals();
-#         endif
-        }
+    if (0 == me) {
+        me = GC_register_my_thread_inner(sb, self);
         me -> flags |= DETACHED;
           /* Treat as detached, since we do not need to worry about     */
           /* pointer results.                                           */
@@ -1424,6 +1413,21 @@ GC_API int GC_CALL GC_register_my_thread(const struct GC_stack_base *sb)
 #       endif
         UNLOCK();
         return GC_SUCCESS;
+    } else if ((me -> flags & FINISHED) != 0) {
+        /* This code is executed when a thread is registered from the   */
+        /* client thread key destructor.                                */
+        GC_record_stack_base(me, sb);
+        me -> flags &= ~FINISHED; /* but not DETACHED */
+#       ifdef GC_EXPLICIT_SIGNALS_UNBLOCK
+          /* Since this could be executed from a thread destructor,     */
+          /* our signals might be blocked.                              */
+          GC_unblock_gc_signals();
+#       endif
+#       if defined(THREAD_LOCAL_ALLOC)
+          GC_init_thread_local(&(me->tlfs));
+#       endif
+        UNLOCK();
+        return GC_SUCCESS;
     } else {
         UNLOCK();
         return GC_DUPLICATE;

diff --git a/tests/threadkey_test.c b/tests/threadkey_test.c
index 0ae94bb..1ea761d 100644 (file)
--- a/tests/threadkey_test.c
+++ b/tests/threadkey_test.c
@@ -61,7 +61,10 @@ int main (void)
 # endif
   for (i = 0; i < LIMIT; i++) {
     pthread_t t;
-    GC_pthread_create (&t, NULL, entry, NULL);
+    void *res;
+    if (GC_pthread_create (&t, NULL, entry, NULL) == 0
+        && (i & 1) != 0)
+      GC_pthread_join (t, &res);
   }
   return 0;
 }

diff --git a/win32_threads.c b/win32_threads.c
index cd8e8e8..68752fa 100644 (file)
--- a/win32_threads.c
+++ b/win32_threads.c
@@ -733,7 +733,7 @@ GC_API int GC_CALL GC_register_my_thread(const struct GC_stack_base *sb)
 #   ifdef GC_PTHREADS
       /* else */ if ((me -> flags & FINISHED) != 0) {
         GC_record_stack_base(me, sb);
-        me -> flags = (me -> flags & ~FINISHED) | DETACHED;
+        me -> flags &= ~FINISHED; /* but not DETACHED */
 #       ifdef THREAD_LOCAL_ALLOC
           GC_init_thread_local((GC_tlfs)(&me->tlfs));
 #       endif
@@ -761,19 +761,34 @@ GC_API int GC_CALL GC_unregister_my_thread(void)
       /* Can't happen: see GC_use_threads_discovery(). */
       GC_ASSERT(FALSE);
 #   else
+#     ifdef GC_PTHREADS
+        /* FIXME: If not DETACHED then just set FINISHED. */
+#     endif
       /* FIXME: Should we just ignore this? */
       GC_delete_thread(GetCurrentThreadId());
 #   endif
   } else {
+#   if defined(THREAD_LOCAL_ALLOC) || defined(GC_PTHREADS)
+      GC_thread me;
+#   endif
     DWORD thread_id = GetCurrentThreadId();
+
     LOCK();
+#   if defined(THREAD_LOCAL_ALLOC) || defined(GC_PTHREADS)
+      me = GC_lookup_thread_inner(thread_id);
+      GC_ASSERT(!(me -> flags & FINISHED));
+#   endif
 #   if defined(THREAD_LOCAL_ALLOC)
-      {
-        GC_thread me = GC_lookup_thread_inner(thread_id);
-        GC_destroy_thread_local(&(me->tlfs));
-      }
+      GC_destroy_thread_local(&(me->tlfs));
+#   endif
+#   ifdef GC_PTHREADS
+      if ((me -> flags & DETACHED) == 0) {
+        me -> flags |= FINISHED;
+      } else
 #   endif
-    GC_delete_thread(thread_id);
+    /* else */ {
+      GC_delete_thread(thread_id);
+    }
     UNLOCK();
   }
   return GC_SUCCESS;