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;
+}

Reply via email to