fix thread cleanup

* libguile/threads.h: Always declare a scm_i_thread_key, for cleanup
  purposes, in the BUILDING_LIBGUILE case.

* libguile/threads.c (scm_i_thread_key): Init with a cleanup handler, so
  any guile-specific info for a thread can be cleaned up reliably.
  (guilify_self_1): Always set the thread key.
  (do_thread_exit_trampoline, on_thread_exit): Enter guile-mode for the
  guile-mode cleanup handler, and trampoline through a
  gc_call_with_stack_base for reasons explained in the code.
  (init_thread_key, scm_i_init_thread_for_guile): Always init the key.
  (scm_i_with_guile_and_parent): No need for pthread_cancel cleanup
  handlers, as the pthread key destructor will take care of that for
  us.
  (really_launch): Remove needless pthread_exit call with incorrect
  comment.

diff --git a/libguile/threads.c b/libguile/threads.c
index e7347ad..e2e17ac 100644 (file)
--- a/libguile/threads.c
+++ b/libguile/threads.c
@@ -343,6 +343,12 @@ unblock_from_queue (SCM queue)
 /* Getting into and out of guile mode.
  */
 
+/* Key used to attach a cleanup handler to a given thread.  Also, if
+   thread-local storage is unavailable, this key is used to retrieve the
+   current thread with `pthread_getspecific ()'.  */
+scm_i_pthread_key_t scm_i_thread_key;
+
+
 #ifdef SCM_HAVE_THREAD_STORAGE_CLASS
 
 /* When thread-local storage (TLS) is available, a pointer to the
@@ -352,17 +358,7 @@ unblock_from_queue (SCM queue)
    represent.  */
 SCM_THREAD_LOCAL scm_i_thread *scm_i_current_thread = NULL;
 
-# define SET_CURRENT_THREAD(_t)  scm_i_current_thread = (_t)
-
-#else /* !SCM_HAVE_THREAD_STORAGE_CLASS */
-
-/* Key used to retrieve the current thread with `pthread_getspecific ()'.  */
-scm_i_pthread_key_t scm_i_thread_key;
-
-# define SET_CURRENT_THREAD(_t)                                \
-  scm_i_pthread_setspecific (scm_i_thread_key, (_t))
-
-#endif /* !SCM_HAVE_THREAD_STORAGE_CLASS */
+#endif /* SCM_HAVE_THREAD_STORAGE_CLASS */
 
 
 static scm_i_pthread_mutex_t thread_admin_mutex = SCM_I_PTHREAD_MUTEX_INITIALIZER;
@@ -428,7 +424,12 @@ guilify_self_1 (SCM_STACKITEM *base)
   t->exited = 0;
   t->guile_mode = 0;
 
-  SET_CURRENT_THREAD (t);
+  scm_i_pthread_setspecific (scm_i_thread_key, t);
+
+#ifdef SCM_HAVE_THREAD_STORAGE_CLASS
+  /* Cache the current thread in TLS for faster lookup.  */
+  scm_i_current_thread = t;
+#endif
 
   scm_i_pthread_mutex_lock (&thread_admin_mutex);
   t->next_thread = all_threads;
@@ -537,6 +538,22 @@ do_thread_exit (void *v)
   return NULL;
 }
 
+static void *
+do_thread_exit_trampoline (struct GC_stack_base *sb, void *v)
+{
+  void *ret;
+  int registered;
+
+  registered = GC_register_my_thread (sb);
+
+  ret = scm_with_guile (do_thread_exit, v);
+
+  if (registered == GC_SUCCESS)
+    GC_unregister_my_thread ();
+
+  return ret;
+}
+
 static void
 on_thread_exit (void *v)
 {
@@ -551,19 +568,20 @@ on_thread_exit (void *v)
       t->held_mutex = NULL;
     }
 
-  SET_CURRENT_THREAD (v);
+  /* Reinstate the current thread for purposes of scm_with_guile
+     guile-mode cleanup handlers.  Only really needed in the non-TLS
+     case but it doesn't hurt to be consistent.  */
+  scm_i_pthread_setspecific (scm_i_thread_key, t);
 
   /* Ensure the signal handling thread has been launched, because we might be
      shutting it down.  */
   scm_i_ensure_signal_delivery_thread ();
 
   /* Unblocking the joining threads needs to happen in guile mode
-     since the queue is a SCM data structure.  */
-
-  /* Note: Since `do_thread_exit ()' uses allocates memory via `libgc', we
-     assume the GC is usable at this point, and notably that thread-local
-     storage (TLS) hasn't been deallocated yet.  */
-  do_thread_exit (v);
+     since the queue is a SCM data structure.   Trampoline through
+     GC_call_with_stack_base so that the GC works even if it already
+     cleaned up for this thread.  */
+  GC_call_with_stack_base (do_thread_exit_trampoline, v);
 
   /* Removing ourself from the list of all threads needs to happen in
      non-guile mode since all SCM values on our stack become
@@ -590,21 +608,17 @@ on_thread_exit (void *v)
 
   scm_i_pthread_mutex_unlock (&thread_admin_mutex);
 
-  SET_CURRENT_THREAD (NULL);
+  scm_i_pthread_setspecific (scm_i_thread_key, NULL);
 }
 
-#ifndef SCM_HAVE_THREAD_STORAGE_CLASS
-
 static scm_i_pthread_once_t init_thread_key_once = SCM_I_PTHREAD_ONCE_INIT;
 
 static void
 init_thread_key (void)
 {
-  scm_i_pthread_key_create (&scm_i_thread_key, NULL);
+  scm_i_pthread_key_create (&scm_i_thread_key, on_thread_exit);
 }
 
-#endif
-
 /* Perform any initializations necessary to make the current thread
    known to Guile (via SCM_I_CURRENT_THREAD), initializing Guile itself,
    if necessary.
@@ -625,9 +639,7 @@ init_thread_key (void)
 static int
 scm_i_init_thread_for_guile (SCM_STACKITEM *base, SCM parent)
 {
-#ifndef SCM_HAVE_THREAD_STORAGE_CLASS
   scm_i_pthread_once (&init_thread_key_once, init_thread_key);
-#endif
 
   if (SCM_I_CURRENT_THREAD)
     {
@@ -790,9 +802,7 @@ scm_i_with_guile_and_parent (void *(*func)(void *), void *data, SCM parent)
       /* We are in Guile mode.  */
       assert (t->guile_mode);
 
-      scm_i_pthread_cleanup_push (scm_leave_guile_cleanup, NULL);
       res = scm_c_with_continuation_barrier (func, data);
-      scm_i_pthread_cleanup_pop (0);
 
       /* Leave Guile mode.  */
       t->guile_mode = 0;
@@ -880,9 +890,6 @@ really_launch (void *d)
   else
     t->result = scm_catch (SCM_BOOL_T, thunk, handler);
 
-  /* Trigger a call to `on_thread_exit ()'.  */
-  pthread_exit (NULL);
-
   return 0;
 }
 

diff --git a/libguile/threads.h b/libguile/threads.h
index b5e3c21..475af32 100644 (file)
--- a/libguile/threads.h
+++ b/libguile/threads.h
@@ -192,6 +192,10 @@ SCM_API void scm_dynwind_critical_section (SCM mutex);
 
 #ifdef BUILDING_LIBGUILE
 
+/* Though we don't need the key for SCM_I_CURRENT_THREAD if we have TLS,
+   we do use it for cleanup purposes.  */
+SCM_INTERNAL scm_i_pthread_key_t scm_i_thread_key;
+
 # ifdef SCM_HAVE_THREAD_STORAGE_CLASS
 
 SCM_INTERNAL SCM_THREAD_LOCAL scm_i_thread *scm_i_current_thread;
@@ -199,7 +203,6 @@ SCM_INTERNAL SCM_THREAD_LOCAL scm_i_thread *scm_i_current_thread;
 
 # else /* !SCM_HAVE_THREAD_STORAGE_CLASS */
 
-SCM_INTERNAL scm_i_pthread_key_t scm_i_thread_key;
 #  define SCM_I_CURRENT_THREAD                                         \
     ((scm_i_thread *) scm_i_pthread_getspecific (scm_i_thread_key))