summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLudovic Courtès <ludo@gnu.org>2008-11-30 20:26:56 +0100
committerLudovic Courtès <ludo@gnu.org>2008-11-30 20:26:56 +0100
commitcb823e630d6f410bacb463d4c42b263bb5b8ce7c (patch)
tree0cdc79469a54060a5e3257bbb72e37f0a747f64a
parentd051899d17b5263df6c9cf446d8732d7e50b726a (diff)
downloadguile-cb823e630d6f410bacb463d4c42b263bb5b8ce7c.tar.gz
Fix potential deadlock in `make-struct'.
* libguile/struct.c (scm_make_struct): Remove critical section, as suggested by Linas Vepstas <linasvepstas@gmail.com>. See http://lists.gnu.org/archive/html/bug-guile/2008-11/msg00036.html for a discussion.
-rw-r--r--NEWS1
-rw-r--r--libguile/struct.c18
2 files changed, 9 insertions, 10 deletions
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;