Re: Locks and threads

[Ludovic Courtès]

Hi Neil,

Neil Jerram <address@hidden> writes:

> I've started working through the lock ordering and threading issues
> that we have.  My plan is (with 1.8.x)
>
> - first to address problems reported by helgrind (since I think we
>   should take advantage of external tools before adding debug code to
>   Guile internally)
>
> - then to run Linas's define-race program, and address the problems
>   that that shows up (including thread-safe define, hopefully)
>
> - then (or maybe as part of the previous step) to look again at
>   Linas's lock order debugging patch, to see if it would make sense to
>   apply some of that.
>
> Does that sound sensible; have I missed anything?

Yes.  I think Helgrind can be very helpful.

> diff --git a/libguile/threads.c b/libguile/threads.c
> index ba17746..05e01e3 100644
> --- a/libguile/threads.c
> +++ b/libguile/threads.c
> @@ -465,13 +465,19 @@ guilify_self_1 (SCM_STACKITEM *base)
>  
>    scm_i_pthread_setspecific (scm_i_thread_key, t);
>  
> -  scm_i_pthread_mutex_lock (&t->heap_mutex);
> -
> +  /* As soon as this thread adds itself to the global thread list, the
> +     GC may think that it has a stack that needs marking.  Therefore
> +     initialize t->top to be the same as t->base, just in case GC runs
> +     before the thread can lock its heap_mutex for the first time. */
> +  t->top = t->base;
>    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);
> +
> +  /* Enter Guile mode. */
> +  scm_enter_guile (t);

Looks good.  `scm_enter_guile ()' does a bit more than the
`scm_i_pthread_mutex_lock ()' call that was removed, but it appears to
be OK.

Thanks for looking into this!

Ludo'.