Re: srfi-18 requirements

["Julian Graham" <joolean@gmail.com>]

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

On Jan 24, 2008 8:38 PM, Julian Graham <joolean@gmail.com> wrote:
> > - we apply the generic / bug fix patch that you already posted, except
> >   without the extra thread_admin_mutex locking (which I think we
> >   concluded we can't justify) - that will be to HEAD
>
> Agreed, though see below...


Actually, in light of Neil's apt suggestion that we get this thing
back on track, I resolve to stop fussing about deadlocks for the time
being -- find attached the patch described above (1 deadlock -- the
jmpbuf critical section one -- and the thread-specific mutex).  Let me
know if I've missed anything.


Regards,
Julian

[-- Attachment #2: srfi-18-bugs.HEAD.patch.20080127 --]
[-- Type: application/octet-stream, Size: 6450 bytes --]

Index: libguile/ChangeLog
===================================================================
RCS file: /sources/guile/guile/guile-core/libguile/ChangeLog,v
retrieving revision 1.2421
diff -a -u -r1.2421 ChangeLog
--- libguile/ChangeLog	29 Dec 2007 01:35:33 -0000	1.2421
+++ libguile/ChangeLog	28 Jan 2008 01:54:49 -0000
@@ -1,3 +1,13 @@
+2008-01-27  Julian Graham  <joolean@gmail.com>
+
+	* threads.c (do_thread_exit, scm_cancel_thread, 
+	scm_set_thread_cleanup_x, scm_thread_cleanup): Lock on thread-specific 
+	admin mutex instead of `thread_admin_mutex'.
+	* threads.h (scm_i_thread)[admin_mutex]: New field.
+	* throw.c (make_jmpbuf): Don't enter critical section during thread
+	spawn -- there is a possibility of deadlock if other threads are
+	exiting.
+	
 2007-12-29  Neil Jerram  <neil@ossau.uklinux.net>
 
 	* gc.c (mark_gc_async): Change "func_data" to "fn_data", to avoid
Index: libguile/threads.c
===================================================================
RCS file: /sources/guile/guile/guile-core/libguile/threads.c,v
retrieving revision 1.90
diff -a -u -r1.90 threads.c
--- libguile/threads.c	20 Oct 2007 11:09:58 -0000	1.90
+++ libguile/threads.c	28 Jan 2008 01:54:51 -0000
@@ -1,4 +1,4 @@
-/* Copyright (C) 1995,1996,1997,1998,2000,2001, 2002, 2003, 2004, 2005, 2006, 2007 Free Software Foundation, Inc.
+/* Copyright (C) 1995,1996,1997,1998,2000,2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008 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
@@ -435,6 +435,7 @@
   /* XXX - check for errors. */
   pipe (t->sleep_pipe);
   scm_i_pthread_mutex_init (&t->heap_mutex, NULL);
+  scm_i_pthread_mutex_init (&t->admin_mutex, NULL);
   t->clear_freelists_p = 0;
   t->gc_running_p = 0;
   t->canceled = 0;
@@ -494,7 +495,7 @@
 				      scm_handle_by_message_noexit, NULL);
     }
 
-  scm_i_scm_pthread_mutex_lock (&thread_admin_mutex);
+  scm_i_scm_pthread_mutex_lock (&t->admin_mutex);
 
   t->exited = 1;
   close (t->sleep_pipe[0]);
@@ -502,7 +503,7 @@
   while (scm_is_true (unblock_from_queue (t->join_queue)))
     ;
 
-  scm_i_pthread_mutex_unlock (&thread_admin_mutex);
+  scm_i_pthread_mutex_unlock (&t->admin_mutex);
 
   return NULL;
 }
@@ -931,15 +932,15 @@
 
   SCM_VALIDATE_THREAD (1, thread);
   t = SCM_I_THREAD_DATA (thread);
-  scm_i_scm_pthread_mutex_lock (&thread_admin_mutex);
+  scm_i_scm_pthread_mutex_lock (&t->admin_mutex);
   if (!t->canceled)
     {
       t->canceled = 1;
-      scm_i_pthread_mutex_unlock (&thread_admin_mutex);
+      scm_i_pthread_mutex_unlock (&t->admin_mutex);
       scm_i_pthread_cancel (t->pthread);
     }
   else
-    scm_i_pthread_mutex_unlock (&thread_admin_mutex);
+    scm_i_pthread_mutex_unlock (&t->admin_mutex);
 
   return SCM_UNSPECIFIED;
 }
@@ -957,13 +958,13 @@
   if (!scm_is_false (proc))
     SCM_VALIDATE_THUNK (2, proc);
 
-  scm_i_pthread_mutex_lock (&thread_admin_mutex);
-
   t = SCM_I_THREAD_DATA (thread);
+  scm_i_pthread_mutex_lock (&t->admin_mutex);
+
   if (!(t->exited || t->canceled))
     t->cleanup_handler = proc;
 
-  scm_i_pthread_mutex_unlock (&thread_admin_mutex);
+  scm_i_pthread_mutex_unlock (&t->admin_mutex);
 
   return SCM_UNSPECIFIED;
 }
@@ -979,10 +980,10 @@
 
   SCM_VALIDATE_THREAD (1, thread);
 
-  scm_i_pthread_mutex_lock (&thread_admin_mutex);
   t = SCM_I_THREAD_DATA (thread);
+  scm_i_pthread_mutex_lock (&t->admin_mutex);
   ret = (t->exited || t->canceled) ? SCM_BOOL_F : t->cleanup_handler;
-  scm_i_pthread_mutex_unlock (&thread_admin_mutex);
+  scm_i_pthread_mutex_unlock (&t->admin_mutex);
 
   return ret;
 }
@@ -1001,24 +1002,24 @@
   if (scm_is_eq (scm_current_thread (), thread))
     SCM_MISC_ERROR ("cannot join the current thread", SCM_EOL);
 
-  scm_i_scm_pthread_mutex_lock (&thread_admin_mutex);
-
   t = SCM_I_THREAD_DATA (thread);
+  scm_i_scm_pthread_mutex_lock (&t->admin_mutex);
+
   if (!t->exited)
     {
       while (1)
 	{
-	  block_self (t->join_queue, thread, &thread_admin_mutex, NULL);
+	  block_self (t->join_queue, thread, &t->admin_mutex, NULL);
 	  if (t->exited)
 	    break;
-	  scm_i_pthread_mutex_unlock (&thread_admin_mutex);
+	  scm_i_pthread_mutex_unlock (&t->admin_mutex);
 	  SCM_TICK;
-	  scm_i_scm_pthread_mutex_lock (&thread_admin_mutex);
+	  scm_i_scm_pthread_mutex_lock (&t->admin_mutex);
 	}
     }
   res = t->result;
 
-  scm_i_pthread_mutex_unlock (&thread_admin_mutex);
+  scm_i_pthread_mutex_unlock (&t->admin_mutex);
 
   return res;
 }
Index: libguile/threads.h
===================================================================
RCS file: /sources/guile/guile/guile-core/libguile/threads.h,v
retrieving revision 1.49
diff -a -u -r1.49 threads.h
--- libguile/threads.h	20 Oct 2007 11:09:58 -0000	1.49
+++ libguile/threads.h	28 Jan 2008 01:54:51 -0000
@@ -3,7 +3,7 @@
 #ifndef SCM_THREADS_H
 #define SCM_THREADS_H
 
-/* Copyright (C) 1996,1997,1998,2000,2001, 2002, 2003, 2004, 2006, 2007 Free Software Foundation, Inc.
+/* Copyright (C) 1996,1997,1998,2000,2001, 2002, 2003, 2004, 2006, 2007, 2008 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
@@ -52,6 +52,9 @@
 
   SCM cleanup_handler;
   SCM join_queue;
+
+  scm_i_pthread_mutex_t admin_mutex;
+
   SCM result;
   int canceled;
   int exited;
Index: libguile/throw.c
===================================================================
RCS file: /sources/guile/guile/guile-core/libguile/throw.c,v
retrieving revision 1.114
diff -a -u -r1.114 throw.c
--- libguile/throw.c	22 Jan 2007 15:14:40 -0000	1.114
+++ libguile/throw.c	28 Jan 2008 01:54:52 -0000
@@ -1,4 +1,4 @@
-/* Copyright (C) 1995,1996,1997,1998,2000,2001, 2003, 2004, 2006 Free Software Foundation, Inc.
+/* Copyright (C) 1995,1996,1997,1998,2000,2001, 2003, 2004, 2006, 2008 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
@@ -75,12 +75,8 @@
 make_jmpbuf (void)
 {
   SCM answer;
-  SCM_CRITICAL_SECTION_START;
-  {
-    SCM_NEWSMOB2 (answer, tc16_jmpbuffer, 0, 0);
-    SETJBJMPBUF(answer, (jmp_buf *)0);
-    DEACTIVATEJB(answer);
-  }
-  SCM_CRITICAL_SECTION_END;
+  SCM_NEWSMOB2 (answer, tc16_jmpbuffer, 0, 0);
+  SETJBJMPBUF(answer, (jmp_buf *)0);
+  DEACTIVATEJB(answer);
   return answer;
 }