Re: Trouble joining with threads from C

[ludo@gnu.org (Ludovic Courtès)]

[-- Attachment #1: Type: text/plain, Size: 6853 bytes --]

Hello!

Andy Wingo <wingo@pobox.com> writes:

> When do you propose that the cleanup handlers for the thread be called?
>
> As far as I understand things, reliably cleaning up after the thread
> *requires* the use of pthread_key with a destructor.  It is the only way
> to attach a cleanup callback to a thread.  It's true that the key is not
> used when TLS is available, except for its destructor capabilities, but
> it is not more complicated than before.
>
> I think they context that you are missing here is bug 32436.
>
> It does appear that I have introduced some bugs here: Mark's
> after-gc-hook not being called, and a failure to build
> --without-threads.  For that I apologize.  But these bugs are fixable.
> Consider bug 32436 for a moment: you can't fix that with
> pthread_cleanup_push.

Attached are 3 tests: one for Mark’s bug, and two for #32436.

Currently they all pass, but ‘test-scm-spawn-thread’ hits a libgc
assertion failure (“Duplicate large block deallocation”) once every 5
runs or so:

--8<---------------cut here---------------start------------->8---
(gdb) thread apply all bt

Thread 3 (Thread 0x7ffff582e700 (LWP 15207)):
#0  0x00007ffff62b220b in memset () from /nix/store/vxycd107wjbhcj720hzkw2px7s7kr724-glibc-2.12.2/lib/libc.so.6
#1  0x00007ffff6e8c6d8 in GC_generic_malloc (lb=524288, k=<value optimized out>) at ../malloc.c:193
#2  0x00007ffff7b36aad in make_vm () at vm.c:501
#3  0x00007ffff7b36b8e in scm_the_vm () at vm.c:578
#4  0x00007ffff7ac3190 in scm_call_4 (proc=0x778ab0, arg1=<value optimized out>, arg2=<value optimized out>, arg3=<value optimized out>, arg4=<value optimized out>) at eval.c:476
#5  0x00007ffff7aba283 in scm_i_with_continuation_barrier (body=0x7ffff7ab9a90 <c_body>, body_data=0x7ffff582dd80, handler=0x7ffff7ab9e60 <c_handler>, handler_data=0x7ffff582dd80,
    pre_unwind_handler=<value optimized out>, pre_unwind_handler_data=<value optimized out>) at continuations.c:450
#6  0x00007ffff7aba335 in scm_c_with_continuation_barrier (func=<value optimized out>, data=<value optimized out>) at continuations.c:546
#7  0x00007ffff7b3146a in with_guile_and_parent (base=0x7ffff582dde0, data=0x7ffff582de00) at threads.c:856
#8  0x00007ffff6e916b5 in GC_call_with_stack_base (fn=<value optimized out>, arg=<value optimized out>) at ../misc.c:1505
#9  0x00007ffff7b3109c in scm_i_with_guile_and_parent (d=<value optimized out>) at threads.c:899
#10 spawn_thread (d=<value optimized out>) at threads.c:1056
#11 0x00007ffff6e96791 in GC_inner_start_routine (sb=<value optimized out>, arg=<value optimized out>) at ../pthread_start.c:61
#12 0x00007ffff6e916b5 in GC_call_with_stack_base (fn=<value optimized out>, arg=<value optimized out>) at ../misc.c:1505
#13 0x00007ffff6c61cec in start_thread () from /nix/store/vxycd107wjbhcj720hzkw2px7s7kr724-glibc-2.12.2/lib/libpthread.so.0
#14 0x00007ffff63041ed in clone () from /nix/store/vxycd107wjbhcj720hzkw2px7s7kr724-glibc-2.12.2/lib/libc.so.6

Thread 2 (Thread 0x7ffff602f700 (LWP 15206)):
#0  0x00007ffff626585e in raise () from /nix/store/vxycd107wjbhcj720hzkw2px7s7kr724-glibc-2.12.2/lib/libc.so.6
#1  0x00007ffff6266d16 in abort () from /nix/store/vxycd107wjbhcj720hzkw2px7s7kr724-glibc-2.12.2/lib/libc.so.6
#2  0x00007ffff6e90d9f in GC_abort (msg=0x7ffff6e98b50 "Duplicate large block deallocation") at ../misc.c:1409
#3  0x00007ffff6e85b3c in GC_freehblk (hbp=0x980000) at ../allchblk.c:852
#4  0x00007ffff6e977cb in GC_pthread_create (new_thread=0x7ffff602ee38, attr=0x0, start_routine=0x7ffff7b31060 <spawn_thread>, arg=0x7ffff602edb0) at ../pthread_support.c:1552
#5  0x00007ffff7b31cbc in scm_spawn_thread (body=<value optimized out>, body_data=<value optimized out>, handler=0x7ffff7b33f20 <scm_handle_by_message>, handler_data=0x7ffff7b72868) at threads.c:1078
#6  0x00007ffff7b0e1cc in start_signal_delivery_thread () at scmsigs.c:193
#7  0x00007ffff6c67c83 in pthread_once () from /nix/store/vxycd107wjbhcj720hzkw2px7s7kr724-glibc-2.12.2/lib/libpthread.so.0
#8  0x00007ffff7b3115d in on_thread_exit (v=0x96a600) at threads.c:692
#9  0x00007ffff6c61b29 in __nptl_deallocate_tsd () from /nix/store/vxycd107wjbhcj720hzkw2px7s7kr724-glibc-2.12.2/lib/libpthread.so.0
#10 0x00007ffff6c61cfa in start_thread () from /nix/store/vxycd107wjbhcj720hzkw2px7s7kr724-glibc-2.12.2/lib/libpthread.so.0
#11 0x00007ffff63041ed in clone () from /nix/store/vxycd107wjbhcj720hzkw2px7s7kr724-glibc-2.12.2/lib/libc.so.6

Thread 1 (Thread 0x7ffff7ff3700 (LWP 15205)):
#0  0x00007ffff6c6692b in pthread_cond_timedwait@@GLIBC_2.3.2 () from /nix/store/vxycd107wjbhcj720hzkw2px7s7kr724-glibc-2.12.2/lib/libpthread.so.0
#1  0x00007ffff7b31e76 in scm_pthread_cond_timedwait (cond=0x662e88, mutex=0x96a628, wt=0x7fffffffcca0) at threads.c:1916
#2  0x00007ffff7b31fca in block_self (queue=0x981fc0, sleep_object=<value optimized out>, mutex=0x96a628, waittime=0x7fffffffcca0) at threads.c:431
#3  0x00007ffff7b321f9 in scm_join_thread_timed (thread=0x981ff0, timeout=<value optimized out>, timeoutval=<value optimized out>) at threads.c:1205
#4  0x00007ffff7ab9a9a in c_body (d=0x7fffffffcf30) at continuations.c:512
#5  0x00007ffff7b497b0 in vm_regular_engine (vm=0x6f38c0, program=0x7ffff7b72f72, argv=0x6f70a8, nargs=1) at vm-i-system.c:956
#6  0x00007ffff7ac31a3 in scm_call_4 (proc=0x778ab0, arg1=<value optimized out>, arg2=<value optimized out>, arg3=<value optimized out>, arg4=<value optimized out>) at eval.c:476
#7  0x00007ffff7aba283 in scm_i_with_continuation_barrier (body=0x7ffff7ab9a90 <c_body>, body_data=0x7fffffffcf30, handler=0x7ffff7ab9e60 <c_handler>, handler_data=0x7fffffffcf30,
    pre_unwind_handler=<value optimized out>, pre_unwind_handler_data=<value optimized out>) at continuations.c:450
#8  0x00007ffff7aba335 in scm_c_with_continuation_barrier (func=<value optimized out>, data=<value optimized out>) at continuations.c:546
#9  0x00007ffff7b3146a in with_guile_and_parent (base=0x7fffffffcf90, data=0x7fffffffcfb0) at threads.c:856
#10 0x00007ffff6e916b5 in GC_call_with_stack_base (fn=<value optimized out>, arg=<value optimized out>) at ../misc.c:1505
#11 0x00007ffff7b31598 in scm_i_with_guile_and_parent (func=<value optimized out>, data=<value optimized out>) at threads.c:899
#12 scm_with_guile (func=<value optimized out>, data=<value optimized out>) at threads.c:905
#13 0x00000000004008c0 in main (argc=<value optimized out>, argv=<value optimized out>) at test-scm-spawn-thread.c:60
--8<---------------cut here---------------end--------------->8---

This happens when calling ‘GC_pthread_create’ from a pthread key
destructor, which appears to be problematic [0].

I haven’t been able to fix it or otherwise work around it though, so
help welcome!  :-)

I think this is not a showstopper for 2.0.1.

Thanks,
Ludo’.

[0] http://thread.gmane.org/gmane.comp.programming.garbage-collection.boehmgc/4502


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: the tests --]
[-- Type: text/x-patch, Size: 8887 bytes --]

From 257ee9f4449c82c4cf9d3cd9951e48df549667fb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Fri, 22 Apr 2011 12:29:50 +0200
Subject: [PATCH] Add pthread-related tests.

* test-suite/standalone/Makefile.am (test_scm_spawn_thread_CFLAGS,
  test_scm_spawn_thread_LDADD, test_pthread_create_CFLAGS,
  test_pthread_create_LDADD, test_pthread_create_secondary_CFLAGS,
  test_pthread_create_secondary_LDADD): New variables.
  (check_PROGRAMS)[BUILD_PTHREAD_SUPPORT]: Add `test-scm-spawn-thread',
  `test-pthread_create', `test-pthread_create-secondary'.
  (TESTS)[BUILD_PTHREAD_SUPPORT]: Likewise.

* test-suite/standalone/test-scm-spawn-thread.c,
  test-suite/standalone/test-pthread-create.c,
  test-suite/standalone/test-pthread-create-secondary.c: New files.
---
 .gitignore                                         |    3 +
 test-suite/standalone/Makefile.am                  |   15 ++++
 .../standalone/test-pthread-create-secondary.c     |   85 ++++++++++++++++++++
 test-suite/standalone/test-pthread-create.c        |   63 +++++++++++++++
 test-suite/standalone/test-scm-spawn-thread.c      |   62 ++++++++++++++
 5 files changed, 228 insertions(+), 0 deletions(-)
 create mode 100644 test-suite/standalone/test-pthread-create-secondary.c
 create mode 100644 test-suite/standalone/test-pthread-create.c
 create mode 100644 test-suite/standalone/test-scm-spawn-thread.c

diff --git a/.gitignore b/.gitignore
index f24e589..a0eeeca 100644
--- a/.gitignore
+++ b/.gitignore
@@ -139,3 +139,6 @@ INSTALL
 /.sc-start-*
 /lib/math.h
 /lib/sys/time.h
+/test-suite/standalone/test-scm-spawn-thread
+/test-suite/standalone/test-pthread-create
+/test-suite/standalone/test-pthread-create-secondary
diff --git a/test-suite/standalone/Makefile.am b/test-suite/standalone/Makefile.am
index b21edd2..cf1fc4f 100644
--- a/test-suite/standalone/Makefile.am
+++ b/test-suite/standalone/Makefile.am
@@ -198,6 +198,21 @@ test_scm_with_guile_LDADD = $(LIBGUILE_LDADD)
 check_PROGRAMS += test-scm-with-guile
 TESTS += test-scm-with-guile
 
+test_scm_spawn_thread_CFLAGS = ${test_cflags}
+test_scm_spawn_thread_LDADD = $(LIBGUILE_LDADD)
+check_PROGRAMS += test-scm-spawn-thread
+TESTS += test-scm-spawn-thread
+
+test_pthread_create_CFLAGS = ${test_cflags}
+test_pthread_create_LDADD = $(LIBGUILE_LDADD)
+check_PROGRAMS += test-pthread-create
+TESTS += test-pthread-create
+
+test_pthread_create_secondary_CFLAGS = ${test_cflags} $(BDW_GC_CFLAGS)
+test_pthread_create_secondary_LDADD = $(LIBGUILE_LDADD)
+check_PROGRAMS += test-pthread-create-secondary
+TESTS += test-pthread-create-secondary
+
 else
 
 EXTRA_DIST += test-with-guile-module.c test-scm-with-guile.c
diff --git a/test-suite/standalone/test-pthread-create-secondary.c b/test-suite/standalone/test-pthread-create-secondary.c
new file mode 100644
index 0000000..fe39c2a
--- /dev/null
+++ b/test-suite/standalone/test-pthread-create-secondary.c
@@ -0,0 +1,85 @@
+/* Copyright (C) 2011 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
+ * as published by the Free Software Foundation; either version 3 of
+ * the License, or (at your option) any later version.
+ *
+ * This library 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 this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+/* Test whether threads created with `pthread_create' work, and whether
+   a secondary thread can call `scm_with_guile'. (bug #32436).  */
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+#include <pthread.h>
+#include <stdlib.h>
+#include <libguile.h>
+
+#include <gc/gc_version.h>
+
+
+/* Up to GC 7.2alpha5, calling `GC_INIT' from a secondary thread would
+   lead to a segfault.  This was fixed in BDW-GC on 2011-04-16 by Ivan
+   Maidanski.  See <http://thread.gmane.org/gmane.lisp.guile.bugs/5340>
+   for details.  */
+
+#if (GC_VERSION_MAJOR > 7)					\
+  || ((GC_VERSION_MAJOR == 7) && (GC_VERSION_MINOR > 2))	\
+  || ((GC_VERSION_MAJOR == 7) && (GC_VERSION_MINOR == 2)	\
+      && (GC_ALPHA_VERSION > 5))
+
+static void *
+do_something (void *arg)
+{
+  scm_list_copy (scm_make_list (scm_from_int (1234), SCM_BOOL_T));
+  scm_gc ();
+  return NULL;
+}
+
+static void *
+thread (void *arg)
+{
+  scm_with_guile (do_something, NULL);
+  return NULL;
+}
+
+\f
+int
+main (int argc, char *argv[])
+{
+  int i;
+
+  for (i = 0; i < 77; i++)
+    {
+      pthread_t thr;
+
+      pthread_create (&thr, NULL, thread, NULL);
+      pthread_join (thr, NULL);
+    }
+
+  return EXIT_SUCCESS;
+}
+
+\f
+#else /* GC < 7.2 */
+
+int
+main (int argc, char *argv[])
+{
+  /* Skip.  */
+  return 77;
+}
+
+#endif /* GC < 7.2 */
diff --git a/test-suite/standalone/test-pthread-create.c b/test-suite/standalone/test-pthread-create.c
new file mode 100644
index 0000000..8d9617b
--- /dev/null
+++ b/test-suite/standalone/test-pthread-create.c
@@ -0,0 +1,63 @@
+/* Copyright (C) 2011 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
+ * as published by the Free Software Foundation; either version 3 of
+ * the License, or (at your option) any later version.
+ *
+ * This library 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 this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+/* Test whether threads created with `pthread_create' work (bug #32436)
+   when then main thread is the one that initializes Guile.  */
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+#include <pthread.h>
+#include <stdlib.h>
+#include <libguile.h>
+
+static void *
+do_something (void *arg)
+{
+  scm_list_copy (scm_make_list (scm_from_int (1234), SCM_BOOL_T));
+  scm_gc ();
+  return NULL;
+}
+
+static void *
+thread (void *arg)
+{
+  scm_with_guile (do_something, NULL);
+  return NULL;
+}
+
+\f
+int
+main (int argc, char *argv[])
+{
+  int i;
+  pthread_t thr;
+
+  scm_init_guile ();
+
+  do_something (NULL);
+
+  for (i = 0; i < 77; i++)
+    {
+      pthread_create (&thr, NULL, thread, NULL);
+      pthread_join (thr, NULL);
+    }
+
+  return EXIT_SUCCESS;
+}
diff --git a/test-suite/standalone/test-scm-spawn-thread.c b/test-suite/standalone/test-scm-spawn-thread.c
new file mode 100644
index 0000000..b632ab0
--- /dev/null
+++ b/test-suite/standalone/test-scm-spawn-thread.c
@@ -0,0 +1,62 @@
+/* Copyright (C) 2011 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
+ * as published by the Free Software Foundation; either version 3 of
+ * the License, or (at your option) any later version.
+ *
+ * This library 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 this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+/* Test whether a thread created with `scm_spawn_thread' can be joined.
+   See <http://thread.gmane.org/gmane.lisp.guile.devel/11804> for the
+   original report.  */
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+#include <libguile.h>
+
+#include <time.h>
+#include <stdlib.h>
+
+static SCM
+thread_main (void *data)
+{
+  return SCM_BOOL_T;
+}
+
+static SCM
+thread_handler (void *data, SCM key, SCM args)
+{
+  return SCM_BOOL_T;
+}
+
+static void *
+inner_main (void *data)
+{
+  SCM thread, timeout;
+
+  thread = scm_spawn_thread (thread_main, 0, thread_handler, 0);
+  timeout = scm_from_unsigned_integer (time (NULL) + 10);
+  return (void *) scm_join_thread_timed (thread, timeout, SCM_BOOL_F);
+}
+
+\f
+int
+main (int argc, char **argv)
+{
+  SCM result;
+
+  result = PTR2SCM (scm_with_guile (inner_main, 0));
+  return scm_is_true (result) ? EXIT_SUCCESS : EXIT_FAILURE;
+}
-- 
1.7.4.1