Hi Neil, Neil Jerram <n...@ossau.uklinux.net> 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'.
