From ccb80964cd7cd112e300c34d32f67125a6d6da9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= Date: Tue, 28 Jun 2011 15:25:22 +0200 Subject: [PATCH] Fix lock ordering in `fat_mutex_lock' to match that of `do_thread_exit'. Original Helgrind report: ==14160== Thread #57: lock order "0x47F6B90 before 0x7C25A28" violated ==14160== at 0x4C27730: pthread_mutex_lock (in /.../valgrind-3.6.0/lib/valgrind/vgpreload_helgrind-amd64-linux.so) ==14160== by 0x4EFF87E: do_thread_exit (threads.c:664) ==14160== by 0x4E87B89: c_body (continuations.c:512) ==14160== by 0x4F16C21: vm_regular_engine (vm-i-system.c:960) ==14160== by 0x4E90F92: scm_call_4 (eval.c:506) ==14160== by 0x4E88372: scm_i_with_continuation_barrier (continuations.c:450) ==14160== by 0x4E88424: scm_c_with_continuation_barrier (continuations.c:546) ==14160== by 0x51CA3AF: GC_call_with_gc_active (pthread_support.c:1128) ==14160== by 0x4EFF2E0: with_guile_and_parent (threads.c:206) ==14160== by 0x51C46B4: GC_call_with_stack_base (misc.c:1505) ==14160== by 0x4EFF447: scm_with_guile (threads.c:917) ==14160== by 0x51C46B4: GC_call_with_stack_base (misc.c:1505) ==14160== Required order was established by acquisition of lock at 0x47F6B90 ==14160== at 0x4C27730: pthread_mutex_lock (in /.../valgrind-3.6.0/lib/valgrind/vgpreload_helgrind-amd64-linux.so) ==14160== by 0x4F00262: fat_mutex_lock (threads.c:1362) ==14160== by 0x4F004DA: scm_lock_mutex_timed (threads.c:1462) ==14160== by 0x4F16C09: vm_regular_engine (vm-i-system.c:898) ==14160== by 0x4E90F4D: scm_call_3 (eval.c:499) ==14160== by 0x4EFFA4C: really_launch (threads.c:975) ==14160== by 0x4E87B89: c_body (continuations.c:512) ==14160== by 0x4F16C21: vm_regular_engine (vm-i-system.c:960) ==14160== by 0x4E90F92: scm_call_4 (eval.c:506) ==14160== by 0x4E88372: scm_i_with_continuation_barrier (continuations.c:450) ==14160== by 0x4E88424: scm_c_with_continuation_barrier (continuations.c:546) ==14160== by 0x4EFF289: with_guile_and_parent (threads.c:874) ==14160== followed by a later acquisition of lock at 0x7C25A28 ==14160== at 0x4C27730: pthread_mutex_lock (in /.../valgrind-3.6.0/lib/valgrind/vgpreload_helgrind-amd64-linux.so) ==14160== by 0x4F00352: fat_mutex_lock (threads.c:1374) ==14160== by 0x4F004DA: scm_lock_mutex_timed (threads.c:1462) ==14160== by 0x4F16C09: vm_regular_engine (vm-i-system.c:898) ==14160== by 0x4E90F4D: scm_call_3 (eval.c:499) ==14160== by 0x4EFFA4C: really_launch (threads.c:975) ==14160== by 0x4E87B89: c_body (continuations.c:512) ==14160== by 0x4F16C21: vm_regular_engine (vm-i-system.c:960) ==14160== by 0x4E90F92: scm_call_4 (eval.c:506) ==14160== by 0x4E88372: scm_i_with_continuation_barrier (continuations.c:450) ==14160== by 0x4E88424: scm_c_with_continuation_barrier (continuations.c:546) ==14160== by 0x4EFF289: with_guile_and_parent (threads.c:874) * libguile/threads.c (fat_mutex_lock): In the `m->level == 0' case, release M's lock before taking T's `admin_mutex'. --- libguile/threads.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libguile/threads.c b/libguile/threads.c index dba5d16f1..c07c85342 100644 --- a/libguile/threads.c +++ b/libguile/threads.c @@ -1371,6 +1371,8 @@ fat_mutex_lock (SCM mutex, scm_t_timespec *timeout, SCM owner, int *ret) if (SCM_I_IS_THREAD (new_owner)) { scm_i_thread *t = SCM_I_THREAD_DATA (new_owner); + + scm_i_pthread_mutex_unlock (&m->lock); scm_i_pthread_mutex_lock (&t->admin_mutex); /* Only keep a weak reference to MUTEX so that it's not @@ -1381,6 +1383,7 @@ fat_mutex_lock (SCM mutex, scm_t_timespec *timeout, SCM owner, int *ret) t->mutexes = scm_weak_car_pair (mutex, t->mutexes); scm_i_pthread_mutex_unlock (&t->admin_mutex); + scm_i_pthread_mutex_lock (&m->lock); } *ret = 1; break;