From f60a7648d5926555c7760364a6fbb7dc0cf60720 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Fri, 18 Mar 2011 13:18:47 +0100 Subject: 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. --- libguile/threads.c | 71 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 32 deletions(-) (limited to 'libguile/threads.c') diff --git a/libguile/threads.c b/libguile/threads.c index e7347ad57..e2e17acc6 100644 --- 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; } -- cgit v1.2.3 From 5f0d2951a0a5179038bee55fe9af688f94738075 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Thu, 24 Mar 2011 20:34:31 +0100 Subject: bdw-gc 6.8 compatibility (hopefully) * configure.ac (HAVE_GC_STACK_BASE): New check. * libguile/threads.c (GC_UNIMPLEMENTED, GC_SUCCESS): Define if needed. (GC_register_my_thread, GC_unregister_my_thread) (GC_call_with_stack_base): Define shims if needed. --- configure.ac | 7 +++++++ libguile/threads.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) (limited to 'libguile/threads.c') diff --git a/configure.ac b/configure.ac index ba6ff4900..6f24d8db4 100644 --- a/configure.ac +++ b/configure.ac @@ -1256,6 +1256,13 @@ AC_CHECK_TYPE([GC_fn_type], [], [#include ]) +# `GC_stack_base' is not available in GC 7.1 and earlier. +AC_CHECK_TYPE([struct GC_stack_base], + [AC_DEFINE([HAVE_GC_STACK_BASE], [1], + [Define this if the `GC_stack_base' type is available.])], + [], + [#include ]) + LIBS="$save_LIBS" diff --git a/libguile/threads.c b/libguile/threads.c index e2e17acc6..6f75dbe19 100644 --- a/libguile/threads.c +++ b/libguile/threads.c @@ -79,6 +79,37 @@ typedef void * (* GC_fn_type) (void *); #endif +#ifndef GC_SUCCESS +#define GC_SUCCESS 0 +#endif + +#ifndef GC_UNIMPLEMENTED +#define GC_UNIMPLEMENTED 3 +#endif + +/* Likewise struct GC_stack_base is missing before 7.1. */ +#ifndef HAVE_GC_STACK_BASE +struct GC_stack_base; + +static int +GC_register_my_thread (struct GC_stack_base *) +{ + return GC_UNIMPLEMENTED; +} + +static void +GC_unregister_my_thread () +{ +} + +static void * +GC_call_with_stack_base(void * (*fn) (struct GC_stack_base*, void*), void *arg) +{ + return fn (NULL, arg); +} +#endif + + /* Now define with_gc_active and with_gc_inactive. */ #if (defined(HAVE_GC_DO_BLOCKING) && defined (HAVE_DECL_GC_DO_BLOCKING) && defined (HAVE_GC_CALL_WITH_GC_ACTIVE)) -- cgit v1.2.3 From 12c1d8616d8dfedcad65f34e3968f9544b629ae1 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Fri, 25 Mar 2011 13:01:51 +0100 Subject: threading / with_guile refactor to use more GC_stack_base * libguile/init.h: * libguile/init.c (scm_i_init_guile): Change arg to this internal function from SCM_STACKITEM* to void*. Actually it's a struct GC_stack_base*. * libguile/bdw-gc.h: Don't do pthread redirects, because we don't want to affect applications' pthread_* bindings. * libguile/pthread-threads.h (scm_i_pthread_create) (scm_i_pthread_detach, scm_i_pthread_exit, scm_i_pthread_cancel) (scm_i_pthread_sigmask): Do pthread redirects here, in this internal header. * libguile/threads.h: Remove declaration of internal scm_i_with_guile_and_parent. Remove declaration of undefined scm_threads_init_first_thread. Make declaration of internal scm_threads_prehistory actually internal, and take a void* (actually a struct GC_stack_base*). * libguile/threads.c (GC_get_stack_base): Implement a shim if this function is unavailable, and fold in the implementations of get_thread_stack_base. (GC_call_with_stack_base): Actually implement. (guilify_self_1): Take a GC_stack_base* as an arg. (scm_i_init_thread_for_guile): Likewise, and set up libgc for registration of other threads. (scm_init_guile): Use GC_get_stack_base instead of our own guesswork. (with_guile_and_parent, scm_i_with_guile_and_parent): Rework to trampoline through a GC_call_with_stack_base. (scm_threads_prehistory): Pass the "base" arg on to guilify_self_1. --- libguile/bdw-gc.h | 7 +- libguile/init.c | 8 +- libguile/init.h | 4 +- libguile/pthread-threads.h | 14 +-- libguile/threads.c | 272 ++++++++++++++++++++++++--------------------- libguile/threads.h | 8 +- 6 files changed, 162 insertions(+), 151 deletions(-) (limited to 'libguile/threads.c') diff --git a/libguile/bdw-gc.h b/libguile/bdw-gc.h index 3adf99e66..61c11eb94 100644 --- a/libguile/bdw-gc.h +++ b/libguile/bdw-gc.h @@ -1,7 +1,7 @@ #ifndef SCM_BDW_GC_H #define SCM_BDW_GC_H -/* Copyright (C) 2006, 2008, 2009 Free Software Foundation, Inc. +/* Copyright (C) 2006, 2008, 2009, 2011 Free Software Foundation, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public License @@ -32,6 +32,11 @@ # define GC_THREADS 1 # define GC_REDIRECT_TO_LOCAL 1 +/* Don't #define pthread routines to their GC_pthread counterparts. + Instead we will be careful inside Guile to use the GC_pthread + routines. */ +# define GC_NO_THREAD_REDIRECTS 1 + #endif #include diff --git a/libguile/init.c b/libguile/init.c index d6a710504..8b3b8cd33 100644 --- a/libguile/init.c +++ b/libguile/init.c @@ -376,17 +376,11 @@ cleanup_for_exit () } void -scm_i_init_guile (SCM_STACKITEM *base) +scm_i_init_guile (void *base) { if (scm_initialized_p) return; - if (base == NULL) - { - fprintf (stderr, "cannot determine stack base!\n"); - abort (); - } - if (sizeof (mpz_t) > (3 * sizeof (scm_t_bits))) { fprintf (stderr, diff --git a/libguile/init.h b/libguile/init.h index 7cfae76d5..bc6cddf93 100644 --- a/libguile/init.h +++ b/libguile/init.h @@ -3,7 +3,7 @@ #ifndef SCM_INIT_H #define SCM_INIT_H -/* Copyright (C) 1995,1996,1997,2000, 2006, 2008 Free Software Foundation, Inc. +/* Copyright (C) 1995,1996,1997,2000, 2006, 2008, 2011 Free Software Foundation, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public License @@ -38,7 +38,7 @@ SCM_API void scm_boot_guile (int argc, char **argv, char **argv), void *closure); -SCM_INTERNAL void scm_i_init_guile (SCM_STACKITEM *base); +SCM_INTERNAL void scm_i_init_guile (void *base); SCM_API void scm_load_startup_files (void); diff --git a/libguile/pthread-threads.h b/libguile/pthread-threads.h index ca72f1674..c180af252 100644 --- a/libguile/pthread-threads.h +++ b/libguile/pthread-threads.h @@ -3,7 +3,7 @@ #ifndef SCM_PTHREADS_THREADS_H #define SCM_PTHREADS_THREADS_H -/* Copyright (C) 2002, 2005, 2006 Free Software Foundation, Inc. +/* Copyright (C) 2002, 2005, 2006, 2011 Free Software Foundation, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public License @@ -29,24 +29,24 @@ #include #include -/* `libgc' intercepts pthread calls by defining wrapping macros. */ +/* `libgc' defines wrapper procedures for pthread calls. */ #include "libguile/bdw-gc.h" /* Threads */ #define scm_i_pthread_t pthread_t #define scm_i_pthread_self pthread_self -#define scm_i_pthread_create pthread_create -#define scm_i_pthread_detach pthread_detach -#define scm_i_pthread_exit pthread_exit -#define scm_i_pthread_cancel pthread_cancel +#define scm_i_pthread_create GC_pthread_create +#define scm_i_pthread_detach GC_pthread_detach +#define scm_i_pthread_exit GC_pthread_exit +#define scm_i_pthread_cancel GC_pthread_cancel #define scm_i_pthread_cleanup_push pthread_cleanup_push #define scm_i_pthread_cleanup_pop pthread_cleanup_pop #define scm_i_sched_yield sched_yield /* Signals */ -#define scm_i_pthread_sigmask pthread_sigmask +#define scm_i_pthread_sigmask GC_pthread_sigmask /* Mutexes */ diff --git a/libguile/threads.c b/libguile/threads.c index 6f75dbe19..2f5569fb5 100644 --- a/libguile/threads.c +++ b/libguile/threads.c @@ -89,7 +89,12 @@ typedef void * (* GC_fn_type) (void *); /* Likewise struct GC_stack_base is missing before 7.1. */ #ifndef HAVE_GC_STACK_BASE -struct GC_stack_base; +struct GC_stack_base { + void * mem_base; /* Base of memory stack. */ +#ifdef __ia64__ + void * reg_base; /* Base of separate register stack. */ +#endif +}; static int GC_register_my_thread (struct GC_stack_base *) @@ -102,13 +107,93 @@ GC_unregister_my_thread () { } +#if !SCM_USE_PTHREAD_THREADS +/* No threads; we can just use GC_stackbottom. */ static void * -GC_call_with_stack_base(void * (*fn) (struct GC_stack_base*, void*), void *arg) +get_thread_stack_base () +{ + return GC_stackbottom; +} + +#elif defined HAVE_PTHREAD_ATTR_GETSTACK && defined HAVE_PTHREAD_GETATTR_NP \ + && defined PTHREAD_ATTR_GETSTACK_WORKS +/* This method for GNU/Linux and perhaps some other systems. + It's not for MacOS X or Solaris 10, since pthread_getattr_np is not + available on them. */ +static void * +get_thread_stack_base () +{ + pthread_attr_t attr; + void *start, *end; + size_t size; + + pthread_getattr_np (pthread_self (), &attr); + pthread_attr_getstack (&attr, &start, &size); + end = (char *)start + size; + +#if SCM_STACK_GROWS_UP + return start; +#else + return end; +#endif +} + +#elif defined HAVE_PTHREAD_GET_STACKADDR_NP +/* This method for MacOS X. + It'd be nice if there was some documentation on pthread_get_stackaddr_np, + but as of 2006 there's nothing obvious at apple.com. */ +static void * +get_thread_stack_base () +{ + return pthread_get_stackaddr_np (pthread_self ()); +} + +#else +#error Threads enabled with old BDW-GC, but missing get_thread_stack_base impl. Please upgrade to libgc >= 7.1. +#endif + +static int +GC_get_stack_base (struct GC_stack_base *) { - return fn (NULL, arg); + stack_base->mem_base = get_thread_stack_base (); +#ifdef __ia64__ + /* Calculate and store off the base of this thread's register + backing store (RBS). Unfortunately our implementation(s) of + scm_ia64_register_backing_store_base are only reliable for the + main thread. For other threads, therefore, find out the current + top of the RBS, and use that as a maximum. */ + stack_base->reg_base = scm_ia64_register_backing_store_base (); + { + ucontext_t ctx; + void *bsp; + getcontext (&ctx); + bsp = scm_ia64_ar_bsp (&ctx); + if (stack_base->reg_base > bsp) + stack_base->reg_base = bsp; + } +#endif + return GC_SUCCESS; } + +static void * +GC_call_with_stack_base(void * (*fn) (struct GC_stack_base*, void*), void *arg) +{ + struct GC_stack_base stack_base; + + stack_base.mem_base = (void*)&stack_base; +#ifdef __ia64__ + /* FIXME: Untested. */ + { + ucontext_t ctx; + getcontext (&ctx); + stack_base.reg_base = scm_ia64_ar_bsp (&ctx); + } #endif + return fn (&stack_base, arg); +} +#endif /* HAVE_GC_STACK_BASE */ + /* Now define with_gc_active and with_gc_inactive. */ @@ -401,7 +486,7 @@ static SCM scm_i_default_dynamic_state; /* Perform first stage of thread initialisation, in non-guile mode. */ static void -guilify_self_1 (SCM_STACKITEM *base) +guilify_self_1 (struct GC_stack_base *base) { scm_i_thread *t = scm_gc_malloc (sizeof (scm_i_thread), "thread"); @@ -418,25 +503,12 @@ guilify_self_1 (SCM_STACKITEM *base) t->block_asyncs = 1; t->pending_asyncs = 1; t->critical_section_level = 0; - t->base = base; + t->base = base->mem_base; #ifdef __ia64__ - /* Calculate and store off the base of this thread's register - backing store (RBS). Unfortunately our implementation(s) of - scm_ia64_register_backing_store_base are only reliable for the - main thread. For other threads, therefore, find out the current - top of the RBS, and use that as a maximum. */ - t->register_backing_store_base = scm_ia64_register_backing_store_base (); - { - ucontext_t ctx; - void *bsp; - getcontext (&ctx); - bsp = scm_ia64_ar_bsp (&ctx); - if (t->register_backing_store_base > bsp) - t->register_backing_store_base = bsp; - } + t->register_backing_store_base = base->reg-base; #endif t->continuation_root = SCM_EOL; - t->continuation_base = base; + t->continuation_base = t->base; scm_i_pthread_cond_init (&t->sleep_cond, NULL); t->sleep_mutex = NULL; t->sleep_object = SCM_BOOL_F; @@ -668,7 +740,7 @@ init_thread_key (void) be sure. New threads are put into guile mode implicitly. */ static int -scm_i_init_thread_for_guile (SCM_STACKITEM *base, SCM parent) +scm_i_init_thread_for_guile (struct GC_stack_base *base, SCM parent) { scm_i_pthread_once (&init_thread_key_once, init_thread_key); @@ -690,6 +762,10 @@ scm_i_init_thread_for_guile (SCM_STACKITEM *base, SCM parent) initialization. */ scm_i_init_guile (base); + + /* Allow other threads to come in later. */ + GC_allow_register_threads (); + scm_i_pthread_mutex_unlock (&scm_i_init_mutex); } else @@ -698,6 +774,10 @@ scm_i_init_thread_for_guile (SCM_STACKITEM *base, SCM parent) the first time. Only initialize this thread. */ scm_i_pthread_mutex_unlock (&scm_i_init_mutex); + + /* Register this thread with libgc. */ + GC_register_my_thread (base); + guilify_self_1 (base); guilify_self_2 (parent); } @@ -705,135 +785,58 @@ scm_i_init_thread_for_guile (SCM_STACKITEM *base, SCM parent) } } -#if SCM_USE_PTHREAD_THREADS - -#if defined HAVE_PTHREAD_ATTR_GETSTACK && defined HAVE_PTHREAD_GETATTR_NP -/* This method for GNU/Linux and perhaps some other systems. - It's not for MacOS X or Solaris 10, since pthread_getattr_np is not - available on them. */ -#define HAVE_GET_THREAD_STACK_BASE - -static SCM_STACKITEM * -get_thread_stack_base () +void +scm_init_guile () { - pthread_attr_t attr; - void *start, *end; - size_t size; - - pthread_getattr_np (pthread_self (), &attr); - pthread_attr_getstack (&attr, &start, &size); - end = (char *)start + size; - - /* XXX - pthread_getattr_np from LinuxThreads does not seem to work - for the main thread, but we can use scm_get_stack_base in that - case. - */ - -#ifndef PTHREAD_ATTR_GETSTACK_WORKS - if ((void *)&attr < start || (void *)&attr >= end) - return (SCM_STACKITEM *) GC_stackbottom; + struct GC_stack_base stack_base; + + if (GC_get_stack_base (&stack_base) == GC_SUCCESS) + scm_i_init_thread_for_guile (&stack_base, + scm_i_default_dynamic_state); else -#endif { -#if SCM_STACK_GROWS_UP - return start; -#else - return end; -#endif + fprintf (stderr, "Failed to get stack base for current thread.\n"); + exit (1); } } -#elif defined HAVE_PTHREAD_GET_STACKADDR_NP -/* This method for MacOS X. - It'd be nice if there was some documentation on pthread_get_stackaddr_np, - but as of 2006 there's nothing obvious at apple.com. */ -#define HAVE_GET_THREAD_STACK_BASE -static SCM_STACKITEM * -get_thread_stack_base () -{ - return pthread_get_stackaddr_np (pthread_self ()); -} - -#elif defined (__MINGW32__) -/* This method for mingw. In mingw the basic scm_get_stack_base can be used - in any thread. We don't like hard-coding the name of a system, but there - doesn't seem to be a cleaner way of knowing scm_get_stack_base can - work. */ -#define HAVE_GET_THREAD_STACK_BASE -static SCM_STACKITEM * -get_thread_stack_base () -{ - return (SCM_STACKITEM *) GC_stackbottom; -} - -#endif /* pthread methods of get_thread_stack_base */ - -#else /* !SCM_USE_PTHREAD_THREADS */ - -#define HAVE_GET_THREAD_STACK_BASE - -static SCM_STACKITEM * -get_thread_stack_base () -{ - return (SCM_STACKITEM *) GC_stackbottom; -} - -#endif /* !SCM_USE_PTHREAD_THREADS */ - -#ifdef HAVE_GET_THREAD_STACK_BASE - -void -scm_init_guile () -{ - scm_i_init_thread_for_guile (get_thread_stack_base (), - scm_i_default_dynamic_state); -} - -#endif - -void * -scm_with_guile (void *(*func)(void *), void *data) -{ - return scm_i_with_guile_and_parent (func, data, - scm_i_default_dynamic_state); -} - SCM_UNUSED static void scm_leave_guile_cleanup (void *x) { on_thread_exit (SCM_I_CURRENT_THREAD); } -struct with_guile_trampoline_args +struct with_guile_args { GC_fn_type func; void *data; + SCM parent; }; static void * with_guile_trampoline (void *data) { - struct with_guile_trampoline_args *args = data; + struct with_guile_args *args = data; return scm_c_with_continuation_barrier (args->func, args->data); } -void * -scm_i_with_guile_and_parent (void *(*func)(void *), void *data, SCM parent) +static void * +with_guile_and_parent (struct GC_stack_base *base, void *data) { void *res; int new_thread; scm_i_thread *t; - SCM_STACKITEM base_item; + struct with_guile_args *args = data; - new_thread = scm_i_init_thread_for_guile (&base_item, parent); + new_thread = scm_i_init_thread_for_guile (base, args->parent); t = SCM_I_CURRENT_THREAD; if (new_thread) { /* We are in Guile mode. */ assert (t->guile_mode); - res = scm_c_with_continuation_barrier (func, data); + res = scm_c_with_continuation_barrier (args->func, args->data); /* Leave Guile mode. */ t->guile_mode = 0; @@ -841,14 +844,10 @@ scm_i_with_guile_and_parent (void *(*func)(void *), void *data, SCM parent) else if (t->guile_mode) { /* Already in Guile mode. */ - res = scm_c_with_continuation_barrier (func, data); + res = scm_c_with_continuation_barrier (args->func, args->data); } else { - struct with_guile_trampoline_args args; - args.func = func; - args.data = data; - /* We are not in Guile mode, either because we are not within a scm_with_guile, or because we are within a scm_without_guile. @@ -857,20 +856,39 @@ scm_i_with_guile_and_parent (void *(*func)(void *), void *data, SCM parent) when this thread was first guilified. Thus, `base' must be updated. */ #if SCM_STACK_GROWS_UP - if (SCM_STACK_PTR (&base_item) < t->base) - t->base = SCM_STACK_PTR (&base_item); + if (SCM_STACK_PTR (base->mem_base) < t->base) + t->base = SCM_STACK_PTR (base->mem_base); #else - if (SCM_STACK_PTR (&base_item) > t->base) - t->base = SCM_STACK_PTR (&base_item); + if (SCM_STACK_PTR (base->mem_base) > t->base) + t->base = SCM_STACK_PTR (base->mem_base); #endif t->guile_mode = 1; - res = with_gc_active (with_guile_trampoline, &args); + res = with_gc_active (with_guile_trampoline, args); t->guile_mode = 0; } return res; } +static void * +scm_i_with_guile_and_parent (void *(*func)(void *), void *data, SCM parent) +{ + struct with_guile_args args; + + args.func = func; + args.data = data; + args.parent = parent; + + return GC_call_with_stack_base (with_guile_and_parent, &args); +} + +void * +scm_with_guile (void *(*func)(void *), void *data) +{ + return scm_i_with_guile_and_parent (func, data, + scm_i_default_dynamic_state); +} + void * scm_without_guile (void *(*func)(void *), void *data) { @@ -2003,7 +2021,7 @@ pthread_mutexattr_t scm_i_pthread_mutexattr_recursive[1]; #endif void -scm_threads_prehistory (SCM_STACKITEM *base) +scm_threads_prehistory (void *base) { #if SCM_USE_PTHREAD_THREADS pthread_mutexattr_init (scm_i_pthread_mutexattr_recursive); @@ -2016,7 +2034,7 @@ scm_threads_prehistory (SCM_STACKITEM *base) scm_i_pthread_mutex_init (&scm_i_misc_mutex, NULL); scm_i_pthread_cond_init (&wake_up_cond, NULL); - guilify_self_1 (base); + guilify_self_1 ((struct GC_stack_base *) base); } scm_t_bits scm_tc16_thread; diff --git a/libguile/threads.h b/libguile/threads.h index 475af328c..9e44684e1 100644 --- a/libguile/threads.h +++ b/libguile/threads.h @@ -136,13 +136,7 @@ SCM_API SCM scm_spawn_thread (scm_t_catch_body body, void *body_data, SCM_API void *scm_without_guile (void *(*func)(void *), void *data); SCM_API void *scm_with_guile (void *(*func)(void *), void *data); -SCM_INTERNAL void *scm_i_with_guile_and_parent (void *(*func)(void *), - void *data, SCM parent); - - -void scm_threads_prehistory (SCM_STACKITEM *); -void scm_threads_init_first_thread (void); - +SCM_INTERNAL void scm_threads_prehistory (void *); SCM_INTERNAL void scm_init_threads (void); SCM_INTERNAL void scm_init_thread_procs (void); SCM_INTERNAL void scm_init_threads_default_dynamic_state (void); -- cgit v1.2.3 From 7f22442b2af85ea9db89c84fbd3acb6a96ee13fd Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Fri, 25 Mar 2011 15:35:20 +0100 Subject: avoid running GC when SCM_I_CURRENT_THREAD is unset * libguile/threads.c (guilify_self_1): Prevent finalizers from running before SCM_I_CURRENT_THREAD is set. (do_thread_exit_trampoline): Leave the thread in the registered state. (on_thread_exit): Always unregister the thread here. --- libguile/threads.c | 117 ++++++++++++++++++++++++++++------------------------- 1 file changed, 63 insertions(+), 54 deletions(-) (limited to 'libguile/threads.c') diff --git a/libguile/threads.c b/libguile/threads.c index 2f5569fb5..ad5bbe19f 100644 --- a/libguile/threads.c +++ b/libguile/threads.c @@ -488,57 +488,73 @@ static SCM scm_i_default_dynamic_state; static void guilify_self_1 (struct GC_stack_base *base) { - scm_i_thread *t = scm_gc_malloc (sizeof (scm_i_thread), "thread"); - - t->pthread = scm_i_pthread_self (); - t->handle = SCM_BOOL_F; - t->result = SCM_BOOL_F; - t->cleanup_handler = SCM_BOOL_F; - t->mutexes = SCM_EOL; - t->held_mutex = NULL; - t->join_queue = SCM_EOL; - t->dynamic_state = SCM_BOOL_F; - t->dynwinds = SCM_EOL; - t->active_asyncs = SCM_EOL; - t->block_asyncs = 1; - t->pending_asyncs = 1; - t->critical_section_level = 0; - t->base = base->mem_base; + scm_i_thread t; + + /* We must arrange for SCM_I_CURRENT_THREAD to point to a valid value + before allocating anything in this thread, because allocation could + cause GC to run, and GC could cause finalizers, which could invoke + Scheme functions, which need the current thread to be set. */ + + t.pthread = scm_i_pthread_self (); + t.handle = SCM_BOOL_F; + t.result = SCM_BOOL_F; + t.cleanup_handler = SCM_BOOL_F; + t.mutexes = SCM_EOL; + t.held_mutex = NULL; + t.join_queue = SCM_EOL; + t.dynamic_state = SCM_BOOL_F; + t.dynwinds = SCM_EOL; + t.active_asyncs = SCM_EOL; + t.block_asyncs = 1; + t.pending_asyncs = 1; + t.critical_section_level = 0; + t.base = base->mem_base; #ifdef __ia64__ - t->register_backing_store_base = base->reg-base; + t.register_backing_store_base = base->reg-base; #endif - t->continuation_root = SCM_EOL; - t->continuation_base = t->base; - scm_i_pthread_cond_init (&t->sleep_cond, NULL); - t->sleep_mutex = NULL; - t->sleep_object = SCM_BOOL_F; - t->sleep_fd = -1; - - if (pipe (t->sleep_pipe) != 0) + t.continuation_root = SCM_EOL; + t.continuation_base = t.base; + scm_i_pthread_cond_init (&t.sleep_cond, NULL); + t.sleep_mutex = NULL; + t.sleep_object = SCM_BOOL_F; + t.sleep_fd = -1; + + if (pipe (t.sleep_pipe) != 0) /* FIXME: Error conditions during the initialization phase are handled gracelessly since public functions such as `scm_init_guile ()' currently have type `void'. */ abort (); - scm_i_pthread_mutex_init (&t->admin_mutex, NULL); - t->current_mark_stack_ptr = NULL; - t->current_mark_stack_limit = NULL; - t->canceled = 0; - t->exited = 0; - t->guile_mode = 0; + scm_i_pthread_mutex_init (&t.admin_mutex, NULL); + t.current_mark_stack_ptr = NULL; + t.current_mark_stack_limit = NULL; + t.canceled = 0; + t.exited = 0; + t.guile_mode = 0; - scm_i_pthread_setspecific (scm_i_thread_key, t); + /* The switcheroo. */ + { + scm_i_thread *t_ptr = &t; + + GC_disable (); + t_ptr = GC_malloc (sizeof (scm_i_thread)); + memcpy (t_ptr, &t, sizeof t); + + scm_i_pthread_setspecific (scm_i_thread_key, t_ptr); #ifdef SCM_HAVE_THREAD_STORAGE_CLASS - /* Cache the current thread in TLS for faster lookup. */ - scm_i_current_thread = t; + /* Cache the current thread in TLS for faster lookup. */ + scm_i_current_thread = t_ptr; #endif - scm_i_pthread_mutex_lock (&thread_admin_mutex); - t->next_thread = all_threads; - all_threads = t; - thread_count++; - scm_i_pthread_mutex_unlock (&thread_admin_mutex); + scm_i_pthread_mutex_lock (&thread_admin_mutex); + t_ptr->next_thread = all_threads; + all_threads = t_ptr; + thread_count++; + scm_i_pthread_mutex_unlock (&thread_admin_mutex); + + GC_enable (); + } } /* Perform second stage of thread initialisation, in guile mode. @@ -644,17 +660,10 @@ do_thread_exit (void *v) static void * do_thread_exit_trampoline (struct GC_stack_base *sb, void *v) { - void *ret; - int registered; - - registered = GC_register_my_thread (sb); + /* Won't hurt if we are already registered. */ + GC_register_my_thread (sb); - ret = scm_with_guile (do_thread_exit, v); - - if (registered == GC_SUCCESS) - GC_unregister_my_thread (); - - return ret; + return scm_with_guile (do_thread_exit, v); } static void @@ -680,11 +689,9 @@ on_thread_exit (void *v) 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. 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); + /* Scheme-level thread finalizers and other cleanup needs to happen in + guile mode. */ + GC_call_with_stack_base (do_thread_exit_trampoline, t); /* Removing ourself from the list of all threads needs to happen in non-guile mode since all SCM values on our stack become @@ -712,6 +719,8 @@ on_thread_exit (void *v) scm_i_pthread_mutex_unlock (&thread_admin_mutex); scm_i_pthread_setspecific (scm_i_thread_key, NULL); + + GC_unregister_my_thread (); } static scm_i_pthread_once_t init_thread_key_once = SCM_I_PTHREAD_ONCE_INIT; -- cgit v1.2.3 From 4000d0641f8a2282a9c52eea33ad91c76625d906 Mon Sep 17 00:00:00 2001 From: Michael Gran Date: Tue, 29 Mar 2011 21:25:04 -0700 Subject: check for GC_allow_register_threads This is not present in earlier versions of BDW-GC * configure.ac: check for GC_allow_register_threads * libguile/threads.c (scm_i_init_thread_for_guile): Only call GC_allow_register_threads if it is present. --- configure.ac | 2 +- libguile/threads.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) (limited to 'libguile/threads.c') diff --git a/configure.ac b/configure.ac index e5b83798a..4fc25536b 100644 --- a/configure.ac +++ b/configure.ac @@ -1238,7 +1238,7 @@ save_LIBS="$LIBS" LIBS="$BDW_GC_LIBS $LIBS" CFLAGS="$BDW_GC_CFLAGS $CFLAGS" -AC_CHECK_FUNCS([GC_do_blocking GC_call_with_gc_active GC_pthread_exit GC_pthread_cancel]) +AC_CHECK_FUNCS([GC_do_blocking GC_call_with_gc_active GC_pthread_exit GC_pthread_cancel GC_allow_register_threads]) # Though the `GC_do_blocking ()' symbol is present in GC 7.1, it is not # declared, and has a different type (returning void instead of diff --git a/libguile/threads.c b/libguile/threads.c index ad5bbe19f..8dc3414dc 100644 --- a/libguile/threads.c +++ b/libguile/threads.c @@ -772,8 +772,10 @@ scm_i_init_thread_for_guile (struct GC_stack_base *base, SCM parent) */ scm_i_init_guile (base); +#ifdef HAVE_GC_ALLOW_REGISTER_THREADS /* Allow other threads to come in later. */ GC_allow_register_threads (); +#endif scm_i_pthread_mutex_unlock (&scm_i_init_mutex); } -- cgit v1.2.3 From 653ccd78fa6770c692f5a737ad70de5aedbfcc7f Mon Sep 17 00:00:00 2001 From: Michael Gran Date: Tue, 29 Mar 2011 21:27:54 -0700 Subject: don't GC unregister null thread GC_unregister_my_thread is only supposed to be called from a thread other than the main thread, so, it should never be called when the system is compiled with null threads. * libguile/threads.c (on_thread_exit)[SCM_USE_NULL_THREADS]: don't call GC_unregister_my_thread --- libguile/threads.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'libguile/threads.c') diff --git a/libguile/threads.c b/libguile/threads.c index 8dc3414dc..d81f4f4ef 100644 --- a/libguile/threads.c +++ b/libguile/threads.c @@ -720,7 +720,9 @@ on_thread_exit (void *v) scm_i_pthread_setspecific (scm_i_thread_key, NULL); +#if !SCM_USE_NULL_THREADS GC_unregister_my_thread (); +#endif } static scm_i_pthread_once_t init_thread_key_once = SCM_I_PTHREAD_ONCE_INIT; -- cgit v1.2.3 From 987b8160f58185c50c4cf4703eb15f04f6cd9f89 Mon Sep 17 00:00:00 2001 From: Kevin Fletcher Date: Thu, 31 Mar 2011 22:16:54 +0200 Subject: fix gc_register_my_thread et al fallback impls * libguile/threads.c (GC_register_my_thread, GC_get_stack_base): Fix fallback impls. --- libguile/threads.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'libguile/threads.c') diff --git a/libguile/threads.c b/libguile/threads.c index d81f4f4ef..14bda1d2f 100644 --- a/libguile/threads.c +++ b/libguile/threads.c @@ -97,7 +97,7 @@ struct GC_stack_base { }; static int -GC_register_my_thread (struct GC_stack_base *) +GC_register_my_thread (struct GC_stack_base *stack_base) { return GC_UNIMPLEMENTED; } @@ -153,7 +153,7 @@ get_thread_stack_base () #endif static int -GC_get_stack_base (struct GC_stack_base *) +GC_get_stack_base (struct GC_stack_base *stack_base) { stack_base->mem_base = get_thread_stack_base (); #ifdef __ia64__ -- cgit v1.2.3