Neil Jerram <n...@ossau.uklinux.net> writes: > next it's on to the real problem, threadsafe define.
I've been running Linas's define-race test program, and hitting the `throw from within critical section' quite a lot. We've already discussed this a couple of times: http://www.mail-archive.com/bug-gu...@gnu.org/msg04613.html http://www.mail-archive.com/guile-devel@gnu.org/msg03016.html Andy proposed just removing the check for 1.8.x, but I would prefer to keep it if possible, as I think there probably still are genuine problems where the check could help. What about the patch below? Thanks, Neil
>From 2daf18f36c6428cbc883c0eb1a381ffbba59614b Mon Sep 17 00:00:00 2001 From: Neil Jerram <n...@ossau.uklinux.net> Date: Tue, 10 Mar 2009 23:55:31 +0000 Subject: [PATCH] Fix spurious `throw from within critical section' errors The crux of this problem was that the thread doing a throw, and so checking scm_i_critical_section_level, was different from the thread that was in a critical section. * libguile/async.h (scm_i_critical_section_level): Removed, replaced by per-thread critical_section_level. (SCM_CRITICAL_SECTION_START, SCM_CRITICAL_SECTION_END): Use per-thread critical_section_level. * libguile/continuations.c (scm_dynthrow): Check per-thread critical_section_level. * libguile/threads.c (guilify_self_1): Init per-thread critical_section_level. (scm_i_critical_section_level): Removed. * libguile/threads.h (scm_i_thread): New critical_section_level field. * libguile/throw.c (scm_ithrow): Check per-thread critical_section_level. --- libguile/async.h | 8 +++----- libguile/continuations.c | 2 +- libguile/threads.c | 2 +- libguile/threads.h | 3 +++ libguile/throw.c | 2 +- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/libguile/async.h b/libguile/async.h index a81a98d..3c1725f 100644 --- a/libguile/async.h +++ b/libguile/async.h @@ -58,20 +58,18 @@ void scm_dynwind_unblock_asyncs (void); the manual. */ -/* Defined in threads.c. scm_i_critical_section_level is only used - for error checking and will go away eventually. */ +/* Defined in threads.c. */ extern scm_i_pthread_mutex_t scm_i_critical_section_mutex; -extern int scm_i_critical_section_level; #define SCM_CRITICAL_SECTION_START \ do { \ scm_i_pthread_mutex_lock (&scm_i_critical_section_mutex);\ SCM_I_CURRENT_THREAD->block_asyncs++; \ - scm_i_critical_section_level++; \ + SCM_I_CURRENT_THREAD->critical_section_level++; \ } while (0) #define SCM_CRITICAL_SECTION_END \ do { \ - scm_i_critical_section_level--; \ + SCM_I_CURRENT_THREAD->critical_section_level--; \ SCM_I_CURRENT_THREAD->block_asyncs--; \ scm_i_pthread_mutex_unlock (&scm_i_critical_section_mutex); \ scm_async_click (); \ diff --git a/libguile/continuations.c b/libguile/continuations.c index 74bb911..69d2569 100644 --- a/libguile/continuations.c +++ b/libguile/continuations.c @@ -256,7 +256,7 @@ scm_dynthrow (SCM cont, SCM val) SCM_STACKITEM *dst = thread->continuation_base; SCM_STACKITEM stack_top_element; - if (scm_i_critical_section_level) + if (thread->critical_section_level) { fprintf (stderr, "continuation invoked from within critical section.\n"); abort (); diff --git a/libguile/threads.c b/libguile/threads.c index 05e01e3..fc3e607 100644 --- a/libguile/threads.c +++ b/libguile/threads.c @@ -422,6 +422,7 @@ guilify_self_1 (SCM_STACKITEM *base) t->active_asyncs = SCM_EOL; t->block_asyncs = 1; t->pending_asyncs = 1; + t->critical_section_level = 0; t->last_debug_frame = NULL; t->base = base; #ifdef __ia64__ @@ -1667,7 +1668,6 @@ scm_i_thread_sleep_for_gc () /* This mutex is used by SCM_CRITICAL_SECTION_START/END. */ scm_i_pthread_mutex_t scm_i_critical_section_mutex; -int scm_i_critical_section_level = 0; static SCM dynwind_critical_section_mutex; diff --git a/libguile/threads.h b/libguile/threads.h index 9417b88..2b0e067 100644 --- a/libguile/threads.h +++ b/libguile/threads.h @@ -113,6 +113,9 @@ typedef struct scm_i_thread { scm_t_contregs *pending_rbs_continuation; #endif + /* Whether this thread is in a critical section. */ + int critical_section_level; + } scm_i_thread; #define SCM_I_IS_THREAD(x) SCM_SMOB_PREDICATE (scm_tc16_thread, x) diff --git a/libguile/throw.c b/libguile/throw.c index 7e2803f..92c5a1a 100644 --- a/libguile/throw.c +++ b/libguile/throw.c @@ -692,7 +692,7 @@ scm_ithrow (SCM key, SCM args, int noreturn SCM_UNUSED) SCM dynpair = SCM_UNDEFINED; SCM winds; - if (scm_i_critical_section_level) + if (SCM_I_CURRENT_THREAD->critical_section_level) { fprintf (stderr, "throw from within critical section.\n"); abort (); -- 1.5.6.5