diff --git a/NEWS b/NEWS index 0e6e0816d..c0020af9d 100644 --- a/NEWS +++ b/NEWS @@ -84,6 +84,7 @@ available: Guile is now always configured in "maintainer mode". same thread ** The handler of SRFI-34 `with-exception-handler' is now invoked in the dynamic environment of the call to `raise' +** Fix potential deadlock in `make-struct' Changes in 1.8.5 (since 1.8.4) diff --git a/libguile/struct.c b/libguile/struct.c index 511fca4c1..cae0f31d0 100644 --- a/libguile/struct.c +++ b/libguile/struct.c @@ -449,8 +449,14 @@ SCM_DEFINE (scm_make_struct, "make-struct", 2, 0, 1, if (! SCM_LAYOUT_TAILP (SCM_CHAR (last_char))) goto bad_tail; } - - SCM_CRITICAL_SECTION_START; + + /* In guile 1.8.5 and earlier, everything below was covered by a + CRITICAL_SECTION lock. This can lead 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, so the critical section has been removed. */ + if (SCM_STRUCT_DATA (vtable)[scm_struct_i_flags] & SCM_STRUCTF_ENTITY) { data = scm_alloc_struct (basic_size + tail_elts, @@ -466,15 +472,7 @@ SCM_DEFINE (scm_make_struct, "make-struct", 2, 0, 1, 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 - finds an invalid type for a "u" field then there's an error throw in a - critical section, which results in an abort(). Not sure if we need any - protection across scm_struct_init. The data array contains garbage at - this point, but until we return it's not visible to anyone except - `gc'. */ scm_struct_init (handle, layout, data, tail_elts, init); return handle;