Subject: [PATCH]: deadlock in make_struct() Date: 16 Nov 2008 To: address@hidden Hi, Here's a deadlock patch. When committing this patch, please copy the text below into the source code commit message, as it provides a record of what the patch does, and why it was made. --linas The patch below fixes a deadlock in the multi-threading code. It fixes this by simply removing the CRITICAL_SECTION lock. There does not seem to be any need for a lock on this section, since all variables accessed are local, and all other literals are compile-time constants. A typical deadlock that was witnessed was: thread 7 -- holding critical section lock, sleeping on &scm_i_sweep_mutex held in scm_make_struct() at struct.c:463, sleeping in increase_mtrigger() at gc-malloc.c:234 thread 5 -- holding heap_mutex, sleeping on critical section held because thread is in guile mode sleeping in scm_c_catch() at throw.c:187 thread 12 -- holding &scm_i_sweep_mutex, sleeping on heap_mutex held in increase_mtrigger() at gc-malloc.c:234 sleeping in scm_i_thread_put_to_sleep() at threads.c:1621 Signed-off-by: Linas Vepstas --- libguile/struct.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) Index: guile-1.8.5/libguile/struct.c =================================================================== --- guile-1.8.5.orig/libguile/struct.c 2008-11-16 10:58:31.000000000 -0600 +++ guile-1.8.5/libguile/struct.c 2008-11-16 12:17:05.000000000 -0600 @@ -450,7 +450,14 @@ SCM_DEFINE (scm_make_struct, "make-struc goto bad_tail; } - SCM_CRITICAL_SECTION_START; + /* In guile 1.8.5 and earlier, everything below was covered by a + CRITICAL_SECTION lock. This leads to deadlocks in garbage collection, + since other threads might be holding the heap_mutex, while sleeping + on the CRITICAL_SECTION lock. There does not seem to be any need + for a lock on the section below, as it does not access or update + any globals. vtable, basic_size, tail_elts are all local variables, + scm_tc3_struct and scm_struct_i_* are all compile-time consts. + So the lock has been removed. */ if (SCM_STRUCT_DATA (vtable)[scm_struct_i_flags] & SCM_STRUCTF_ENTITY) { data = scm_alloc_struct (basic_size + tail_elts, @@ -466,7 +473,6 @@ SCM_DEFINE (scm_make_struct, "make-struc handle = scm_double_cell ((((scm_t_bits) SCM_STRUCT_DATA (vtable)) + scm_tc3_struct), (scm_t_bits) data, 0, 0); - SCM_CRITICAL_SECTION_END; /* In guile 1.8.1 and earlier, the SCM_CRITICAL_SECTION_END above covered also the following scm_struct_init. But that meant if scm_struct_init