Re: Patch to fix segfaults in join-thread

[Dirk Herrmann]

On Thu, 28 Sep 2000 address@hidden wrote:

> In guile 1.4, join-thread segfaults when joining a dead thread. 
> 
> The following patch fixes this, it is diff -u against coop.c in libguile
> --- coop.c~   Mon Jun 12 13:43:35 2000
> +++ coop.c    Wed Sep  6 09:15:10 2000
> @@ -722,8 +722,11 @@
>    coop_t *old, *newthread;
>    
>    /* Check if t is already finished */
> +  if (t == NULL)
> +    return;
>    if (t->base == NULL)
>      return;
> 
> 
>    /* Create a join list if necessary */
>    if (t->joining == NULL)
> [
> To see the problem try the following
> (let ((a (begin-thread (+ 1 1))))
> (sleep 1)
> (join-thread a)
> )


Thanks for the patch.  However, the corresponding code seems to be 
somewhat outdated anyway:  t->base is not set to NULL any more if a thread
is finished.  (The corresponding code is commented.)  Thus, I suggest the
following alternative patch:

Index: libguile/coop-threads.c
===================================================================
RCS file: /cvs/guile/guile-core/libguile/coop-threads.c,v
retrieving revision 1.24
diff -u -r1.24 coop-threads.c
--- libguile/coop-threads.c     2000/09/26 18:37:26     1.24
+++ libguile/coop-threads.c     2000/11/01 09:14:25
@@ -380,8 +380,18 @@
 scm_join_thread (SCM t)
 #define FUNC_NAME s_join_thread
 {
-  SCM_VALIDATE_THREAD (1,t);
-  coop_join (SCM_THREAD_DATA (t));
+  SCM_VALIDATE_THREAD (1, t);
+  /* Dirk:FIXME:: SCM_THREAD_DATA is a handle for a thread.  It may be that a
+   * certain thread implementation uses a value of 0 as a valid thread handle.
+   * With the following code, this thread would always be considered finished.
+   */
+  /* Dirk:FIXME:: With preemptive threading, a thread may finish immediately
+   * after SCM_THREAD_DATA is read.  Thus, it must be guaranteed that the
+   * handle remains valid until the thread-object is garbage collected, or
+   * a mutex has to be used for reading and modifying SCM_THREAD_DATA.
+   */
+  if (SCM_THREAD_DATA (t))
+    coop_join (SCM_THREAD_DATA (t));
   return SCM_BOOL_T;
 }
 #undef FUNC_NAME
Index: libguile/coop.c
===================================================================
RCS file: /cvs/guile/guile-core/libguile/coop.c,v
retrieving revision 1.25
diff -u -r1.25 coop.c
--- libguile/coop.c     2000/04/21 14:16:30     1.25
+++ libguile/coop.c     2000/11/01 09:14:25
@@ -698,11 +698,6 @@
 {
   coop_t *oldthread = (coop_t *) old;
 
-#if 0
-  /* Marking old->base NULL indicates that this thread is dead */
-  oldthread->base = NULL;
-#endif
-
   if (oldthread->specific)
     free (oldthread->specific);
 #ifndef GUILE_PTHREAD_COMPAT
@@ -721,10 +716,6 @@
 {
   coop_t *old, *newthread;
   
-  /* Check if t is already finished */
-  if (t->base == NULL)
-    return;
-
   /* Create a join list if necessary */
   if (t->joining == NULL)
     {

Comments?

Best regards
Dirk