scm_gc_unprotect during GC

[Han-Wen Nienhuys]

Investigations in the signal 6 problem that I reported recently,
pointed me to a call of scm_gc_unprotect_object, which does

        SCM_REDEFER_INTS

and

        SCM_ALLOW_INTS

The last call sets SCM_CURRENT_THREAD->top to NULL, which triggers an
assertion failure later on in scm_threads_mark_stacks.

A short term solution is to make sure that LilyPond never does
scm_gc_unprotect_object() from smobbed destructor. However, I feel
that this is a kludge.

Either scm_gc_unprotect_object is callable during sweep, or this is
forbidden. At present, GUILE does not print a warning, but barfs in an
unrelated location.

I vote that scm_gc_unprotect_object() should be callable. I assume
that the proper solution would be to check whether the current thread
is already suspended.  A patch is below; it also catches wrongly
balanced resume and suspend calls.

Unfortunately,

         make check

barfs, due to the thread marker marking a NULL location during
unif.test - I haven't figured out how to run the tests in isolation,
so I stop here.  As I am very unfamiliar with multi-threaded coding, I
would appreciate comments whether this is a viable approach


--- threads.c.~1.66.~   2004-09-04 02:02:31.000000000 +0200
+++ threads.c   2004-09-11 03:15:17.138205672 +0200
@@ -116,7 +116,8 @@
   scm_t_thread thread;
   SCM result;
   int exited;
-
+  int suspend_count;
+  
   /* For keeping track of the stack and registers. */
   SCM_STACKITEM *base;
   SCM_STACKITEM *top;
@@ -134,6 +135,11 @@
   t->handle = z;
   t->result = creation_protects;
   t->base = NULL;
+
+  /*
+    threads are suspended on creation.
+   */
+  t->suspend_count = 1;
   scm_i_plugin_cond_init (&t->sleep_cond, 0);
   scm_i_plugin_mutex_init (&t->heap_mutex, &scm_i_plugin_mutex);
   t->clear_freelists_p = 0;
@@ -194,16 +200,25 @@
   scm_setspecific (scm_i_root_state_key, data);
   t->root = (scm_root_state *)data;
 }
-  
+#define SUSPENDCOUNT  
 static void
 resume (scm_thread *t)
 {
-  t->top = NULL;
-  if (t->clear_freelists_p)
+#ifdef SUSPENDCOUNT
+  t->suspend_count--;
+
+  assert(t->suspend_count >= 0);
+  
+  if (t->suspend_count == 0)
+#endif
     {
-      *SCM_FREELIST_LOC (scm_i_freelist) = SCM_EOL;
-      *SCM_FREELIST_LOC (scm_i_freelist2) = SCM_EOL;
-      t->clear_freelists_p = 0;
+      t->top = NULL;
+      if (t->clear_freelists_p)
+       {
+         *SCM_FREELIST_LOC (scm_i_freelist) = SCM_EOL;
+         *SCM_FREELIST_LOC (scm_i_freelist2) = SCM_EOL;
+         t->clear_freelists_p = 0;
+       }
     }
 }
 
@@ -219,12 +234,19 @@
 {
   scm_thread *c = SCM_CURRENT_THREAD;
 
-  /* record top of stack for the GC */
-  c->top = SCM_STACK_PTR (&c);
-  /* save registers. */
-  SCM_FLUSH_REGISTER_WINDOWS;
-  setjmp (c->regs);
+#ifdef SUSPENDCOUNT
+  if (c->suspend_count == 0)
+#endif
+    {
+      /* record top of stack for the GC */
+      c->top = SCM_STACK_PTR (&c);
+      /* save registers. */
+      SCM_FLUSH_REGISTER_WINDOWS;
+      setjmp (c->regs);
+
+    }
 
+  c->suspend_count ++;
   return c;
 }
 
@@ -947,7 +969,7 @@
          continue;
        }
 
-      if (t->top == NULL)
+      if (t->suspend_count == 0) // (t->top == NULL)
        {
          /* Thread has not been suspended, which should never happen.
           */
@@ -1241,6 +1263,7 @@
   /* Allocate a fake thread object to be used during bootup. */
   t = malloc (sizeof (scm_thread));
   t->base = NULL;
+  t->suspend_count = 1;
   t->clear_freelists_p = 0;
   scm_i_plugin_mutex_init (&t->heap_mutex, &scm_i_plugin_mutex);
   scm_setspecific (scm_i_thread_key, t);

-- 

 Han-Wen Nienhuys   |   address@hidden   |   http://www.xs4all.nl/~hanwen