Hi!
Andy Wingo <wi...@pobox.com> skribis:
> The solution is, besides just avoiding fork() and threads, to take locks
> on all interesting data structures when you fork(). Fortunately there
> are not too many, and most locks are not nested, so it seems to be a
> doable thing. In wip-threads-and-fork, I added a scm_c_atfork interface
> to define functions to call before and after a fork.
I finally looked in more details at the branch. In particular, this
part looks great):
7329a5db * use scm_c_atfork_lock_static_mutex for guile's static mutexen
585eb4f7 * add scm_c_atfork_lock_static_mutex
9f6ac5d7 * add atfork interface
What about merging these in stable-2.0? (With additional comments–e.g.,
to describe ‘scm_c_atfork’–and correct indentation, and without the
weak-set part of 9f6ac5d7.)
However, these patches don’t seem to handle the specific issue that
‘signal_delivery_thread’ could be holding the allocation lock when
‘fork’ runs. What about doing something along these lines:
diff --git a/libguile/scmsigs.c b/libguile/scmsigs.c
index 86fce0f..e062f24 100644
--- a/libguile/scmsigs.c
+++ b/libguile/scmsigs.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 1995,1996,1997,1998,1999,2000,2001, 2002, 2004, 2006, 2007, 2008, 2009, 2011 Free Software Foundation, Inc.
+/* Copyright (C) 1995,1996,1997,1998,1999,2000,2001, 2002, 2004, 2006, 2007, 2008, 2009, 2011, 2012 Free Software Foundation, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public License
@@ -183,6 +183,12 @@ read_signal_pipe_data (void * data)
return NULL;
}
+/* Lock to protect async queuing. Since `system-async-mark' conses, and
+ thus acquires the GC lock, this lock ensures that the signal thread
+ doesn't call `system-async-mark' in the middle of a `fork'. */
+static scm_i_pthread_mutex_t signal_async_mutex =
+ SCM_I_PTHREAD_MUTEX_INITIALIZER;
+
static SCM
signal_delivery_thread (void *data)
{
@@ -212,7 +218,11 @@ signal_delivery_thread (void *data)
h = SCM_SIMPLE_VECTOR_REF (signal_handler_asyncs, sig);
t = SCM_SIMPLE_VECTOR_REF (signal_handler_threads, sig);
if (scm_is_true (h))
+ {
+ scm_i_pthread_mutex_lock (&signal_async_mutex);
scm_system_async_mark_for_thread (h, t);
+ scm_i_pthread_mutex_unlock (&signal_async_mutex);
+ }
}
else if (sigdata.n == 0)
break; /* the signal pipe was closed. */
@@ -713,6 +723,8 @@ scm_init_scmsigs ()
signal_handler_asyncs = scm_c_make_vector (NSIG, SCM_BOOL_F);
signal_handler_threads = scm_c_make_vector (NSIG, SCM_BOOL_F);
+ scm_c_atfork_lock_static_mutex (&signal_async_mutex);
+
for (i = 0; i < NSIG; i++)
{
#ifdef HAVE_SIGACTION
Thanks,
Ludo’.
