(no subject)

[Neil Jerram]

address@hidden (Ludovic Courtès) writes:

>>> This is normally somewhat fixed in current BDW-GC CVS, but Guile itself
>>> may have troubles of its own dealing with cancellation.
>>
>> Thanks.  I thought already about cancellation, but I couldn't see
>> anything in the test program that would do that.  Do you know of
>> anything in that program, or in BDW-GC, that does a pthread_cancel?
>
> Hmm no, sorry for the confusion.  The srfi-18 test this was taken from
> did use cancellation, though.

OK, thanks for clarifying that.  I guess there must be something other
than cancellation lurking.

> Good that you noticed it was missing from ‘master’.  (BTW ‘signals.test’
> should be changed to GPLv3+ and have a ‘define-module’ clause.)

Thanks, done.

>> commit 3009b5d557e1ebfd828fd0de9b1da5abb2f6ec9a
>> Author: Neil Jerram <address@hidden>
>> Date:   Tue Mar 10 23:55:31 2009 +0000
>>
>>     Fix spurious `throw from within critical section' errors
>>
>> (This is the commit that I hope will fix the errors that you're seeing.)
>>
>> Can you see how the test runs now?
>
> The abort () is apparently never reached with this patch.
>
> It took me some time to understand this:
>
> -  if (scm_i_critical_section_level)
> +  if (thread->critical_section_level)
>      {
>        fprintf (stderr, "continuation invoked from within critical 
> section.\n");
>        abort ();
>
> Critical sections concern all the threads in guile mode.  Thus I would
> think that the question “are the threads in a critical section?” cannot
> be answered by looking at a per-thread ‘critical_section_level’.

That's not the question here.  The question is "is _this_ thread in a
critical section?"  There's no reason (AFAIK) why thread A shouldn't
throw while thread B is in a critical section.

Note that access to the critical section is controlled by a mutex, not
by (scm_i_)critical_section_level.  (scm_i_)critical_section_level only
exists to catch genuine "throw from critical section" errors.

> However, it really seems that the intent was really to forbid ‘throw’s
> when the *current thread* is holding the critical section mutex, so the
> patch is correct but I find the name ‘critical_section_level’ is
> slightly misleading.

The tricky detail here is that scm_i_critical_section_mutex is
initialized as a recursive mutex, so the implication is that it's
possible for a thread to enter the critical section multiple times.
Hence we need some kind of "level", instead of just a boolean
"in_critical_section".

> What do you think?

I have no objection to a more intuitive name, if you can suggest one.

>> Is that right?
>
> Yes.

Thanks.

> BTW, rest assured: thanks to libgc we won’t get any Helgrind report!
> :-)

I assume you mean because most of the helgrind reports involved the
heap_mutex, which no longer exists.  Presumably we could still (in
theory) get helgrind reports for other mutexes?

Regards,
        Neil