[PATCH]: Critical section checked for incorrect thread.

[PATCH]: Critical section checked for incorrect thread.

[Linas Vepstas]

[-- Attachment #1: Type: text/plain, Size: 1277 bytes --]

The patch below was previously submitted:
http://www.mail-archive.com/bug-guile@gnu.org/msg04618.html
but was never applied, I don't know why; there was no
'yea' or 'nay' response.  Please apply; this provides at
least some stability for multi-threaded operation.

--linas

At any given time, some thread might be in a critical section.
Some other thread may be throwing an error for some utterly
unrelated reason.  Yet, when the error is thrown, the
"critical section" check will trip, and it will do so for an
utterly bogus reason. This patch performs the critical section
check, for only for the current thread.

Signed-off-by: Linas Vepstas <linasvepstas@gmail.com>

---
 libguile/throw.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: guile-1.8.6/libguile/throw.c
===================================================================
--- guile-1.8.6.orig/libguile/throw.c  2008-12-08 11:42:56.000000000 -0600
+++ guile-1.8.6/libguile/throw.c 2008-12-13 17:28:27.000000000 -0600
@@ -692,7 +692,7 @@ scm_ithrow (SCM key, SCM args, int noret
   SCM dynpair = SCM_UNDEFINED;
   SCM winds;

-  if (scm_i_critical_section_level)
+  if (SCM_I_CURRENT_THREAD->block_asyncs)
     {
       fprintf (stderr, "throw from within critical section.\n");
       abort ();

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: crit-section.patch --]
[-- Type: text/x-diff; name=crit-section.patch, Size: 1210 bytes --]

Date: 13 Nov 2008
To: guile-devel@gnu.org
Subject: Re: Does anyone actually use threads with guile?

Missed 1.8.6, resubmit on 13 Dec 2008
Subject: [PATCH]: Critical section checked for incorrect thread.

At any given time, some thread might be in a critical section. 
Some other thread may be throwing an error for some utterly
unrelated reason.  Yet, when the error is thrown, the 
"critical section" check will trip, and it will do so for an
utterly bogus reason. This patch performs the critical section
check, for only for the current thread.

Signed-off-by: Linas Vepstas <linasvepstas@gmail.com>

---
 libguile/throw.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: guile-1.8.6/libguile/throw.c
===================================================================
--- guile-1.8.6.orig/libguile/throw.c	2008-12-08 11:42:56.000000000 -0600
+++ guile-1.8.6/libguile/throw.c	2008-12-13 17:28:27.000000000 -0600
@@ -692,7 +692,7 @@ scm_ithrow (SCM key, SCM args, int noret
   SCM dynpair = SCM_UNDEFINED;
   SCM winds;
 
-  if (scm_i_critical_section_level)
+  if (SCM_I_CURRENT_THREAD->block_asyncs)
     {
       fprintf (stderr, "throw from within critical section.\n");
       abort ();

Re: [PATCH]: Critical section checked for incorrect thread.

[Ludovic Courtès]

Hi Linas,

"Linas Vepstas" <linasvepstas@gmail.com> writes:

> --- guile-1.8.6.orig/libguile/throw.c  2008-12-08 11:42:56.000000000 -0600
> +++ guile-1.8.6/libguile/throw.c 2008-12-13 17:28:27.000000000 -0600
> @@ -692,7 +692,7 @@ scm_ithrow (SCM key, SCM args, int noret
>    SCM dynpair = SCM_UNDEFINED;
>    SCM winds;
>
> -  if (scm_i_critical_section_level)
> +  if (SCM_I_CURRENT_THREAD->block_asyncs)
>      {
>        fprintf (stderr, "throw from within critical section.\n");
>        abort ();

It seems to me that "throw from within critical section" means precisely
"throw when SCM_I_CRITICAL_SECTION_LEVEL is non-zero".  I don't see what
asyncs have to do with that.

Thanks,
Ludo'.

Re: [PATCH]: Critical section checked for incorrect thread.

[Andy Wingo]

On Sun 14 Dec 2008 00:59, ludo@gnu.org (Ludovic Courtès) writes:

>> -  if (scm_i_critical_section_level)
>> +  if (SCM_I_CURRENT_THREAD->block_asyncs)
>
> It seems to me that "throw from within critical section" means precisely
> "throw when SCM_I_CRITICAL_SECTION_LEVEL is non-zero".  I don't see what
> asyncs have to do with that.

When scm_i_critical_section_level is nonzero, t->block_asyncs is
nonzero. The reverse is not true.

So... two options then: make this change, hoping that we don't get too
many throws when asyncs are blocked for other reasons... sounds not so
good. Or we remove the check entirely, hoping that there's not too much
erroneous code out there that this check would help. It seems like we
should do the latter, and in 1.9 make it a goal to remove critical
sections entirely, all 45 uses or so, replacing them with finer-grained
mutexen or redoing the code.

Andy
-- 
http://wingolog.org/