From: Neil Jerram <neil@ossau.uklinux.net>
To: Guile Development <guile-devel@gnu.org>
Cc: Linas Vepstas <linasvepstas@gmail.com>
Subject: Re: Locks and threads
Date: Tue, 10 Mar 2009 23:57:01 +0000 [thread overview]
Message-ID: <87vdqhc4oi.fsf@arudy.ossau.uklinux.net> (raw)
In-Reply-To: <87vdqovofz.fsf@arudy.ossau.uklinux.net> (Neil Jerram's message of "Wed\, 04 Mar 2009 23\:49\:20 +0000")
[-- Attachment #1: Type: text/plain, Size: 617 bytes --]
Neil Jerram <neil@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-guile@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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-spurious-throw-from-within-critical-section-er.patch --]
[-- Type: text/x-diff, Size: 4178 bytes --]
From 2daf18f36c6428cbc883c0eb1a381ffbba59614b Mon Sep 17 00:00:00 2001
From: Neil Jerram <neil@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
next prev parent reply other threads:[~2009-03-10 23:57 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-02-11 22:31 Locks and threads Neil Jerram
2009-02-11 23:05 ` Neil Jerram
2009-02-11 23:32 ` Ludovic Courtès
2009-02-11 23:30 ` Linas Vepstas
2009-02-11 23:53 ` Neil Jerram
2009-02-12 0:18 ` Linas Vepstas
2009-02-12 20:51 ` Ludovic Courtès
2009-02-11 23:30 ` Ludovic Courtès
2009-02-12 12:55 ` Greg Troxel
2009-02-12 18:00 ` Ken Raeburn
2009-02-12 21:14 ` Ludovic Courtès
2009-02-14 1:25 ` Ken Raeburn
2009-02-14 16:09 ` Ludovic Courtès
2009-03-05 20:41 ` Neil Jerram
2009-03-04 23:49 ` Neil Jerram
2009-03-05 3:54 ` Linas Vepstas
2009-03-05 19:46 ` Neil Jerram
2009-03-05 20:05 ` Neil Jerram
2009-03-05 20:40 ` Linas Vepstas
2009-03-05 20:49 ` Neil Jerram
2009-03-05 20:57 ` Linas Vepstas
2009-03-05 21:25 ` Neil Jerram
2009-03-05 21:56 ` Linas Vepstas
2009-03-06 11:01 ` Andy Wingo
2009-03-06 12:36 ` Linas Vepstas
2009-03-06 22:05 ` Ludovic Courtès
2009-03-08 22:04 ` Neil Jerram
2009-03-25 19:00 ` Neil Jerram
2009-03-25 22:08 ` Ludovic Courtès
2009-03-05 21:35 ` Ludovic Courtès
2009-03-10 23:57 ` Neil Jerram [this message]
2009-03-12 0:07 ` Neil Jerram
2009-03-12 0:53 ` Neil Jerram
2009-03-12 1:29 ` Linas Vepstas
2009-03-12 3:09 ` Clinton Ebadi
2009-03-25 22:13 ` Neil Jerram
2009-03-25 22:34 ` Linas Vepstas
2009-03-12 22:13 ` Andy Wingo
2009-03-13 19:13 ` Neil Jerram
2009-03-25 23:19 ` Neil Jerram
2009-03-26 3:40 ` Linas Vepstas
2009-03-26 8:02 ` Neil Jerram
2009-03-26 18:39 ` Linas Vepstas
2009-03-26 9:10 ` Ludovic Courtès
2009-03-26 22:01 ` Neil Jerram
2009-03-26 23:12 ` Ludovic Courtès
2009-03-26 22:51 ` Neil Jerram
2009-03-27 3:15 ` Linas Vepstas
2009-03-14 14:23 ` Ludovic Courtès
2009-03-16 22:57 ` Andy Wingo
2009-03-25 18:57 ` Neil Jerram
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/guile/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87vdqhc4oi.fsf@arudy.ossau.uklinux.net \
--to=neil@ossau.uklinux.net \
--cc=guile-devel@gnu.org \
--cc=linasvepstas@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).