lloda pushed a commit to branch main
in repository guile.
commit b0cb76f3d7aabb2491bdca4052408870dc1168b5
Author: Rob Browning <[email protected]>
AuthorDate: Wed Sep 3 15:47:26 2025 -0500
Don't use stale default_dynamic_state null pointer during init
Previously when multiple threads were competing to initialize guile via
scm_with_guile in scm_i_init_thread_for_guile, they would all enter with
a (SCM) 0L dynamic_state (via static initialization) that they captured
from default_dynamic_state in scm_with_guile.
One of them will get the scm_i_init_mutex and initialize guile, which
will set the default_dynamic_state to SCM_BOOL_F via scm_init_threads.
When the thread that initialized guile releases scm_i_init_mutex, the
other threads will proceed, pass the stale 0L to guilify_self_2, and
then segfault in the is_dynamic_state call in
scm_set_current_dynamic_state.
To avoid the problem, statically initialize the default_dynamic_state to
SCM_BOOL_F rather than waiting for scm_init_threads to do it so that we
have one less state to consider, and have all threads that aren't the
one that initializes guile (and the default_dynamic_state) refresh their
dynamic_state whenever it is SCM_BOOL_F.
Add an initial standalone/test-concurrent-init.
Thanks to a aa for reporting the problem, and Dale P. Smith for help
with review and testing.
Closes: 79100
* libguile/threads.c (default_dynamic_state): Statically initialize to
SCM_BOOL_F;
(scm_i_init_thread_for_guile): Refresh dynamic_state when it's SCM_BOOL_F.
(scm_init_threads): Stop initializing default_dynamic_state.
test-suite/Makefile.am: (test_concurrent_init_CFLAGS,
test_concurrent_init_LDADD): New variables.
(check_PROGRAMS): Add test-concurrent-init.
(TESTS): Add test-concurrent-init.
test-suite/standalone/test-concurrent-init.c: New file.
---
libguile/threads.c | 8 +++-
test-suite/standalone/Makefile.am | 5 +++
test-suite/standalone/test-concurrent-init.c | 61 ++++++++++++++++++++++++++++
3 files changed, 72 insertions(+), 2 deletions(-)
diff --git a/libguile/threads.c b/libguile/threads.c
index 6b4510d53..52191128d 100644
--- a/libguile/threads.c
+++ b/libguile/threads.c
@@ -368,7 +368,7 @@ static scm_i_pthread_mutex_t thread_admin_mutex =
SCM_I_PTHREAD_MUTEX_INITIALIZE
static scm_thread *all_threads = NULL;
static int thread_count;
-static SCM default_dynamic_state;
+static SCM default_dynamic_state = SCM_BOOL_F;
/* Perform first stage of thread initialisation, in non-guile mode.
*/
@@ -595,6 +595,11 @@ scm_i_init_thread_for_guile (struct GC_stack_base *base,
needs_unregister = 1;
#endif
+ // We were blocked while another thread was initializing
+ // guile, so we need to re-read the (now correct) state.
+ if (scm_is_eq (dynamic_state, SCM_BOOL_F))
+ dynamic_state = default_dynamic_state;
+
guilify_self_1 (base, needs_unregister);
guilify_self_2 (dynamic_state);
}
@@ -1827,7 +1832,6 @@ scm_init_threads ()
sizeof (struct scm_cond));
scm_set_smob_print (scm_tc16_condvar, scm_cond_print);
- default_dynamic_state = SCM_BOOL_F;
guilify_self_2 (scm_i_make_initial_dynamic_state ());
threads_initialized_p = 1;
diff --git a/test-suite/standalone/Makefile.am
b/test-suite/standalone/Makefile.am
index 5c3b5569e..15d978801 100644
--- a/test-suite/standalone/Makefile.am
+++ b/test-suite/standalone/Makefile.am
@@ -286,6 +286,11 @@ test_pthread_create_secondary_LDADD = $(LIBGUILE_LDADD)
$(top_builddir)/lib/libg
check_PROGRAMS += test-pthread-create-secondary
TESTS += test-pthread-create-secondary
+test_concurrent_init_CFLAGS = ${test_cflags}
+test_concurrent_init_LDADD = $(LIBGUILE_LDADD) $(top_builddir)/lib/libgnu.la
+check_PROGRAMS += test-concurrent-init
+TESTS += test-concurrent-init
+
else
EXTRA_DIST += test-with-guile-module.c test-scm-with-guile.c
diff --git a/test-suite/standalone/test-concurrent-init.c
b/test-suite/standalone/test-concurrent-init.c
new file mode 100644
index 000000000..b26a25a0a
--- /dev/null
+++ b/test-suite/standalone/test-concurrent-init.c
@@ -0,0 +1,61 @@
+/* Copyright 2025
+ Free Software Foundation, Inc.
+
+ This file is part of Guile.
+
+ Guile is free software: you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published
+ by the Free Software Foundation, either version 3 of the License, or
+ (at your option) any later version.
+
+ Guile is distributed in the hope that it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public
+ License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with Guile. If not, see
+ <https://www.gnu.org/licenses/>. */
+
+#if HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+#include <libguile.h>
+
+#include <assert.h>
+#include <pthread.h>
+
+static void *
+guile_main (void *arg)
+{
+ scm_display (scm_cons (scm_from_latin1_symbol ("hello"),
+ scm_from_uintptr_t ((uintptr_t) arg)),
+ scm_current_output_port ());
+ scm_newline (scm_current_output_port ());
+ return arg;
+}
+
+static void *
+thread_main (void *arg)
+{
+ return scm_with_guile (guile_main, arg);
+}
+
+int
+main (int argc, char *argv[])
+{
+ pthread_t t[3];
+
+ for (int i = 0; i < 3; i++)
+ pthread_create (&t[i], NULL, thread_main, t + i);
+
+ for (int i = 0; i < 3; i++)
+ {
+ void *result;
+ pthread_join (t[i], &result);
+ assert (result == t + i);
+ }
+
+ return EXIT_SUCCESS;
+}