Re: Locks and threads

[Neil Jerram]

Neil Jerram <address@hidden> writes:

> - first to address problems reported by helgrind (since I think we
>   should take advantage of external tools before adding debug code to
>   Guile internally)

Here's the next one.

   Neil

>From 76f55c5796f1fc7aca6c36bc57f06bab72300a94 Mon Sep 17 00:00:00 2001
From: Neil Jerram <address@hidden>
Date: Wed, 11 Feb 2009 23:03:16 +0000
Subject: [PATCH] Don't leave Guile mode to lock async_mutex

This fixes lots of helgrind-reported problems, such as:

Thread #1: lock order "0x4325084 before 0x4108928" violated
   at 0x40234F7: pthread_mutex_lock (hg_intercepts.c:408)
   by 0x40CFD37: scm_enter_guile (threads.c:377)
   by 0x40D0284: scm_pthread_mutex_lock (threads.c:1485)
   by 0x405A736: scm_async_click (async.c:155)
   by 0x406F9EE: deval (eval.c:4080)
   by 0x40761D9: scm_primitive_eval_x (eval.c:5921)
   by 0x40AD20E: install_handler (scmsigs.c:113)
   by 0x40AD402: scm_sigaction_for_thread (scmsigs.c:394)
   by 0x4087D1F: scm_gsubr_apply (gsubr.c:223)
   by 0x406DF55: scm_dapply (eval.c:4930)
   by 0x407147C: deval (eval.c:4378)
   by 0x406E1BD: scm_dapply (eval.c:5012)
  Required order was established by acquisition of lock at 0x4325084
   at 0x40234F7: pthread_mutex_lock (hg_intercepts.c:408)
   by 0x40CFD37: scm_enter_guile (threads.c:377)
   by 0x408C58B: scm_i_init_guile (init.c:421)
   by 0x40D1873: scm_i_init_thread_for_guile (threads.c:589)
   by 0x40D18B4: scm_i_with_guile_and_parent (threads.c:731)
   by 0x40D19BD: scm_with_guile (threads.c:720)
   by 0x408C42E: scm_boot_guile (init.c:350)
   by 0x8048710: main (guile.c:69)
  followed by a later acquisition of lock at 0x4108928
   at 0x40234F7: pthread_mutex_lock (hg_intercepts.c:408)
   by 0x40AFD61: scm_make_smob_type (smob.c:294)
   by 0x40AFF19: scm_smob_prehistory (smob.c:512)
   by 0x408C595: scm_i_init_guile (init.c:423)
   by 0x40D1873: scm_i_init_thread_for_guile (threads.c:589)
   by 0x40D18B4: scm_i_with_guile_and_parent (threads.c:731)
   by 0x40D19BD: scm_with_guile (threads.c:720)
   by 0x408C42E: scm_boot_guile (init.c:350)
   by 0x8048710: main (guile.c:69)

* libguile/async.c (scm_async_click): Don't leave Guile mode when
  locking async_mutex.  We don't need to, because none of the code
  that has async_mutex locked can block, and doing so may lead to lock
  ordering problems between async_mutex and a thread's heap_mutex.
---
 libguile/async.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/libguile/async.c b/libguile/async.c
index a9763da..56634d5 100644
--- a/libguile/async.c
+++ b/libguile/async.c
@@ -152,7 +152,7 @@ scm_async_click ()
      invoked even when pending_asyncs is zero.
   */
 
-  scm_i_scm_pthread_mutex_lock (&async_mutex);
+  scm_i_pthread_mutex_lock (&async_mutex);
   t->pending_asyncs = 0;
   if (t->block_asyncs == 0)
     {
@@ -197,7 +197,7 @@ scm_i_queue_async_cell (SCM c, scm_i_thread *t)
   int sleep_fd;
   SCM p;
   
-  scm_i_scm_pthread_mutex_lock (&async_mutex);
+  scm_i_pthread_mutex_lock (&async_mutex);
   p = t->active_asyncs;
   SCM_SETCDR (c, SCM_EOL);
   if (!scm_is_pair (p))
@@ -263,7 +263,7 @@ scm_i_setup_sleep (scm_i_thread *t,
 {
   int pending;
 
-  scm_i_scm_pthread_mutex_lock (&async_mutex);
+  scm_i_pthread_mutex_lock (&async_mutex);
   pending = t->pending_asyncs;
   if (!pending)
     {
@@ -278,7 +278,7 @@ scm_i_setup_sleep (scm_i_thread *t,
 void
 scm_i_reset_sleep (scm_i_thread *t)
 {
-  scm_i_scm_pthread_mutex_lock (&async_mutex);
+  scm_i_pthread_mutex_lock (&async_mutex);
   t->sleep_object = SCM_BOOL_F;
   t->sleep_mutex = NULL;
   t->sleep_fd = -1;
-- 
1.5.6.5