hacking on 1.7 threads

hacking on 1.7 threads

[Julian Graham]

Hi guys,
  I'm trying to debug this thread cancellation patch I wrote against
CVS HEAD, and I'm having a bit of trouble sorting out the source
files.
  The first thing I noticed was that it didn't look like a new Pthread
was being created when I called call-with-new-thread from my Scheme
code; after looking in the Makefile, it looks like none of
threads-plugin.c, pthread-threads.c, and null-threads.c are getting
compiled -- they're all in this list of EXTRA_DIST files.  Am I
missing something?  How can the new threads system get used if it's
not even getting built in?
  Also, I noticed that coop-pthreads.c has a more recent CVS
modification time (if I'm reading the CVS log right) than threads.c,
even though the comments in both files lead me to believe that
threads.c is the future.  Am I missing something?  The way I figured
the new threading stuff was going to work was:

  - threads.c: implements generic thread-manipulation stuff, like
creating and freeing the scm_thread structure, etc.
  - threads-plugin.c, [threading-library]-threads.h: defines some
wrappers for the actual threading-library functions, such as the
scm_i_plugin_* functions.
  - [threading-library]-threads.c: contains any functions that need to
be implemented using the actual API from the chosen threading library.
 This is probably only a few things in most cases, since
threads-plugin maps the plugin functions directly to threading API
calls -- e.g., scm_i_plugin_thread_self -> pthread_self.

  Any corrections?


Cheers,
Julian


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: hacking on 1.7 threads

[Julian Graham]

Okay, I've worked out the files.  But now I've got a different problem
with my implementation, and maybe you guys can give me some insight
because I've got some experience writing code *with* Guile, but none
debugging Guile itself.

The way my thread cancellation patch works is that I add an SCM member
to the scm_thread struct.  The SCM is a list of expressions to be
evaluated if the thread is cancelled.  I use the native threading
library's cancellation / cleanup handler mechanism -- in this case,
pthread_cancel, pthread_cleanup_push / pop.  So in fact, there is only
one cleanup handler, which evaluates each expression in the list of
Scheme "cleanup expressions."  The Scheme cleanup pushing and popping
functions I provide simply push and pop expressions onto and off of
the SCM list stored in the scm_thread struct.

My problem is that when I call cancel on a running thread and the
cancellation handler that's supposed to evaluate the contents of the
cleanup expression list runs, the list no longer contains what I
expect; in fact, it's not a list at all any more sometimes.  Something
happens in between where my cleanup expression pushing function gets
called (I check the list before and after pushing an expression onto
it) and where the cancellation handler gets called.  Could it be some
kind of garbage-collection thing?  My understanding of garbage
collection of SCMs in C code is not wonderful; do I have to do
anything further than scm_remember_upto_here_1 to make sure that the
garbage collector doesn't delete stack data that I use for longer than
its scope?  What about heap data?  That is, if I'm storing a list SCM
in an scm_thread struct that's allocated on the heap, and I cons stuff
onto it, do I have to do anything special to prevent the GC from
collecting the SCM contents of the struct?  Does the fact that it's a
list (i.e., a chain of pairs) that I'm trying to protect make a
difference?  (I'm assuming that if the head element gets marked, all
the CDRs will get marked as well.)


Thanks,
Julian


On Tue, 19 Oct 2004 09:46:20 -0400, Julian Graham <joolean@gmail.com> wrote:
> Hi guys,
>   I'm trying to debug this thread cancellation patch I wrote against
> CVS HEAD, and I'm having a bit of trouble sorting out the source
> files.
>   The first thing I noticed was that it didn't look like a new Pthread
> was being created when I called call-with-new-thread from my Scheme
> code; after looking in the Makefile, it looks like none of
> threads-plugin.c, pthread-threads.c, and null-threads.c are getting
> compiled -- they're all in this list of EXTRA_DIST files.  Am I
> missing something?  How can the new threads system get used if it's
> not even getting built in?
>   Also, I noticed that coop-pthreads.c has a more recent CVS
> modification time (if I'm reading the CVS log right) than threads.c,
> even though the comments in both files lead me to believe that
> threads.c is the future.  Am I missing something?  The way I figured
> the new threading stuff was going to work was:
> 
>   - threads.c: implements generic thread-manipulation stuff, like
> creating and freeing the scm_thread structure, etc.
>   - threads-plugin.c, [threading-library]-threads.h: defines some
> wrappers for the actual threading-library functions, such as the
> scm_i_plugin_* functions.
>   - [threading-library]-threads.c: contains any functions that need to
> be implemented using the actual API from the chosen threading library.
>  This is probably only a few things in most cases, since
> threads-plugin maps the plugin functions directly to threading API
> calls -- e.g., scm_i_plugin_thread_self -> pthread_self.
> 
>   Any corrections?
> 
> Cheers,
> Julian
>


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: hacking on 1.7 threads

[Rob Browning]

Julian Graham <joolean@gmail.com> writes:

> What about heap data?  That is, if I'm storing a list SCM in an
> scm_thread struct that's allocated on the heap, and I cons stuff
> onto it, do I have to do anything special to prevent the GC from
> collecting the SCM contents of the struct?  Does the fact that it's
> a list (i.e., a chain of pairs) that I'm trying to protect make a
> difference?  (I'm assuming that if the head element gets marked, all
> the CDRs will get marked as well.)

If the head is marked, then yes, all the other list elements will be
marked too.

In general, on the C side, only SCM values on the stack will be seen
automatically by the GC.  So any values that are global, static, or
heap allocated must be made visible to the GC explicitly.

This can be done by adding the values to a Scheme side structure
you're already maintaining (one whose lifetime will be longer than the
SCM values in question), or by calling scm_gc_protect_object.  If you
use scm_gc_protect_object, you can call scm_gc_unprotect_object when
you're finished with the value.  The protect and unprotect calls may
be nested, and they'll involve a hash-table operation or so.

There's also a scm_permanent_object function that you can use for
objects that should never go away.  It's a bit cheaper than
scm_gc_protect_object -- basically just something like (set!
*protected-list* (cons obj *protected-list*)).

-- 
Rob Browning
rlb @defaultvalue.org and @debian.org; previously @cs.utexas.edu
GPG starting 2002-11-03 = 14DD 432F AE39 534D B592  F9A0 25C8 D377 8C7E 73A4


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: hacking on 1.7 threads

[Mikael Djurfeldt]

Julian Graham <joolean@gmail.com> writes:

> That is, if I'm storing a list SCM in an scm_thread struct that's
> allocated on the heap, and I cons stuff onto it, do I have to do
> anything special to prevent the GC from collecting the SCM contents
> of the struct?

Yes.  As Rob already has explained, any Scheme object stored in a data
structure allocated outside the stack must be explicitly protected.

The way to do it in this case is to add a line in
threads.c:thread_mark:

static SCM
thread_mark (SCM obj)
{
  scm_thread *t = SCM_THREAD_DATA (obj);
  scm_gc_mark (t->result);
  scm_gc_mark (t->cancellation_handler_list); /* <--- */
  return t->root->handle; /* mark root-state of this thread */
}

Note, though, that this is the easy part.  I do expect that there also
could arise nasty complications having to do with the order in which
things are done at cancellation.  It's for example important that the
scm_thread data structure isn't deallocated before the handlers are
invoked.  It's also important that the GC is still aware of the thread
at that point in time.  It's important that the thread *is* properly
deallocated *after* the handlers have run---that kind of stuff.  But
maybe there's no problem at all.

M


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: hacking on 1.7 threads

[Julian Graham]

Hi Mikael, 
  Yes, I eventually realized on my own that extending the mark
function for threads was the way to go, and I'm currently trying to
address the very synchronization issues you mention below.  I'll have
a look at that document, too.  Thanks!


Julian


On Sun, 24 Oct 2004 11:29:06 +0200, Mikael Djurfeldt
<djurfeldt@nada.kth.se> wrote:
> Julian Graham <joolean@gmail.com> writes:
> 
> > That is, if I'm storing a list SCM in an scm_thread struct that's
> > allocated on the heap, and I cons stuff onto it, do I have to do
> > anything special to prevent the GC from collecting the SCM contents
> > of the struct?
> 
> Yes.  As Rob already has explained, any Scheme object stored in a data
> structure allocated outside the stack must be explicitly protected.
> 
> The way to do it in this case is to add a line in
> threads.c:thread_mark:
> 
> static SCM
> thread_mark (SCM obj)
> {
>   scm_thread *t = SCM_THREAD_DATA (obj);
>   scm_gc_mark (t->result);
>   scm_gc_mark (t->cancellation_handler_list); /* <--- */
>   return t->root->handle; /* mark root-state of this thread */
> }
> 
> Note, though, that this is the easy part.  I do expect that there also
> could arise nasty complications having to do with the order in which
> things are done at cancellation.  It's for example important that the
> scm_thread data structure isn't deallocated before the handlers are
> invoked.  It's also important that the GC is still aware of the thread
> at that point in time.  It's important that the thread *is* properly
> deallocated *after* the handlers have run---that kind of stuff.  But
> maybe there's no problem at all.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: hacking on 1.7 threads

[Julian Graham]

Alright, having combatted the corruption that seems to occur during
the cancellation handler for about a solid straight week and a half,
I'm getting pretty demoralized.  Here's where I am at this point:

- Realized that the GC must be aware of the list of thread cleanup
handler expressions and protected them as part of scm_thread_mark
- The scm_thread data structure removes *itself* from the all_threads
list once it's finished, so I don't think premature deallocation is a
problem
- Realized that the GC might be interrupted by a cancellation signal
in the middle of a collection, since I'm pretty sure it calls
functions that are cancellation points for deferred-cancellation POSIX
threads.  I assume that a half-finished collection could have
disastrous effects for data consistency, so I've taken the stopgap
measure of disabling cancellation while scm_igc() is running.
- It occurs to me that after the cancellation signal is received and a
bunch of pthreads stuff is unwound to call the pthread cancellation
handler, the Scheme evaluation environment for that thread may be in
some unknown state...

...which might explain why I've been getting SIGABRTs and SIGSEGVs
when I call scm_i_eval in my pthread cancellation handler.  Here's a
characteristic stack trace for a SIGABRT

#42 0x40017c2c in ?? ()
#43 0x40b68228 in ?? ()
#44 0x40b681f0 in ?? ()
#45 0x40007def in _dl_lookup_symbol () from /lib/ld-linux.so.2
#46 0x4008e26c in scm_cons (x=0x806e270, y=0x204) at pairs.c:59
#47 0x40058c57 in scm_i_eval (exp=0x806e270, env=0x4031dc40) at eval.c:5859
#48 0x400b4f27 in handler_cancellation (thread=0x80932a8) at threads.c:302
#49 0x4018303b in __pthread_unwind () from /lib/tls/libpthread.so.0
#50 0x4017e4a8 in sigcancel_handler () from /lib/tls/libpthread.so.0

...with many many more ?? stack frames and then a SIGABRT in some
internal libc function.  I can't seem to reproduce the SIGSEGV at the
moment.  I've tried preserving the current evaluation environment in
addition to the expression at the time of the 'push' from Scheme code,
and then evaluating the expression in that saved environment when the
pthread cancellation handler runs, but that doesn't seem to do much
good (though it does raise the question: In what environment should
the cancellation handler expressions be evaluated?  The env. at the
time they were pushed onto the list?  Or the environment at the time
the thread received the cancellation signal?  And what should the
correct error-handling behavior be during evaluation of cleanup
handler expressions?).
  So having tried all this and more with no success, I'm kind of at my
wits' end;  if anyone would like to volunteer to take this code over
from me (it's like 50-60 lines of new code in threads.c,
threads-plugin.c, pthreads-threads.c, and a teensy little bit in
gc.c), I'd be more than happy to comment it up and post the files or a
patch to HEAD.  Or you can rewrite the whole thing from scratch, since
my design may be just plain stupid.


Cheers,
Julian

On Sun, 24 Oct 2004 11:29:06 +0200, Mikael Djurfeldt
<djurfeldt@nada.kth.se> wrote:
> Note, though, that this is the easy part.  I do expect that there also
> could arise nasty complications having to do with the order in which
> things are done at cancellation.  It's for example important that the
> scm_thread data structure isn't deallocated before the handlers are
> invoked.  It's also important that the GC is still aware of the thread
> at that point in time.  It's important that the thread *is* properly
> deallocated *after* the handlers have run---that kind of stuff.  But
> maybe there's no problem at all.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: hacking on 1.7 threads

[Julian Graham]

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

Hi everyone,
  I'm attaching a patch (against HEAD, created in guile/ via 'cvs diff
-Nau') that represents the current state of my work on thread
cancellation (except that I removed the cancellation-disabling stuff
I'd added temporarily to gc.c; I wasn't super confident that it had
any effect).
  I've also attached a little code that demonstrates the functionality
I've added as well as the difficulty I've been having.  Because this
patch doesn't completely work, I haven't included any changes to the
Changelog -- if it's not clear from my previous messages to this list
or from my comments in the code exactly how I've implemented any of
this or what it provides, just drop me a line.


Cheers,
Julian


On Sat, 30 Oct 2004 16:45:14 -0400, Julian Graham <joolean@gmail.com> wrote:
> Alright, having combatted the corruption that seems to occur during
> the cancellation handler for about a solid straight week and a half,
> I'm getting pretty demoralized.  Here's where I am at this point:
> 
> - Realized that the GC must be aware of the list of thread cleanup
> handler expressions and protected them as part of scm_thread_mark
> - The scm_thread data structure removes *itself* from the all_threads
> list once it's finished, so I don't think premature deallocation is a
> problem
> - Realized that the GC might be interrupted by a cancellation signal
> in the middle of a collection, since I'm pretty sure it calls
> functions that are cancellation points for deferred-cancellation POSIX
> threads.  I assume that a half-finished collection could have
> disastrous effects for data consistency, so I've taken the stopgap
> measure of disabling cancellation while scm_igc() is running.
> - It occurs to me that after the cancellation signal is received and a
> bunch of pthreads stuff is unwound to call the pthread cancellation
> handler, the Scheme evaluation environment for that thread may be in
> some unknown state...
> 
> ...which might explain why I've been getting SIGABRTs and SIGSEGVs
> when I call scm_i_eval in my pthread cancellation handler.  Here's a
> characteristic stack trace for a SIGABRT
> 
> #42 0x40017c2c in ?? ()
> #43 0x40b68228 in ?? ()
> #44 0x40b681f0 in ?? ()
> #45 0x40007def in _dl_lookup_symbol () from /lib/ld-linux.so.2
> #46 0x4008e26c in scm_cons (x=0x806e270, y=0x204) at pairs.c:59
> #47 0x40058c57 in scm_i_eval (exp=0x806e270, env=0x4031dc40) at eval.c:5859
> #48 0x400b4f27 in handler_cancellation (thread=0x80932a8) at threads.c:302
> #49 0x4018303b in __pthread_unwind () from /lib/tls/libpthread.so.0
> #50 0x4017e4a8 in sigcancel_handler () from /lib/tls/libpthread.so.0
> 
> ...with many many more ?? stack frames and then a SIGABRT in some
> internal libc function.  I can't seem to reproduce the SIGSEGV at the
> moment.  I've tried preserving the current evaluation environment in
> addition to the expression at the time of the 'push' from Scheme code,
> and then evaluating the expression in that saved environment when the
> pthread cancellation handler runs, but that doesn't seem to do much
> good (though it does raise the question: In what environment should
> the cancellation handler expressions be evaluated?  The env. at the
> time they were pushed onto the list?  Or the environment at the time
> the thread received the cancellation signal?  And what should the
> correct error-handling behavior be during evaluation of cleanup
> handler expressions?).
>   So having tried all this and more with no success, I'm kind of at my
> wits' end;  if anyone would like to volunteer to take this code over
> from me (it's like 50-60 lines of new code in threads.c,
> threads-plugin.c, pthreads-threads.c, and a teensy little bit in
> gc.c), I'd be more than happy to comment it up and post the files or a
> patch to HEAD.  Or you can rewrite the whole thing from scratch, since
> my design may be just plain stupid.
> 
> Cheers,
> 
> 
> Julian
> 
> On Sun, 24 Oct 2004 11:29:06 +0200, Mikael Djurfeldt
> <djurfeldt@nada.kth.se> wrote:
> > Note, though, that this is the easy part.  I do expect that there also
> > could arise nasty complications having to do with the order in which
> > things are done at cancellation.  It's for example important that the
> > scm_thread data structure isn't deallocated before the handlers are
> > invoked.  It's also important that the GC is still aware of the thread
> > at that point in time.  It's important that the thread *is* properly
> > deallocated *after* the handlers have run---that kind of stuff.  But
> > maybe there's no problem at all.
>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: thread-cancellation-HEAD.patch --]
[-- Type: text/x-patch; name="thread-cancellation-HEAD.patch", Size: 14853 bytes --]

Index: guile-core/libguile/Makefile.am
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/Makefile.am,v
retrieving revision 1.195
diff -a -u -r1.195 Makefile.am
--- guile-core/libguile/Makefile.am	24 Sep 2004 02:12:09 -0000	1.195
+++ guile-core/libguile/Makefile.am	7 Nov 2004 02:43:34 -0000
@@ -104,10 +104,11 @@
     lang.c list.c							    \
     load.c macros.c mallocs.c modules.c numbers.c objects.c objprop.c	    \
     options.c pairs.c ports.c print.c procprop.c procs.c properties.c	    \
-    random.c rdelim.c read.c root.c rw.c scmsigs.c script.c simpos.c smob.c \
-    sort.c srcprop.c stackchk.c stacks.c stime.c strings.c srfi-13.c srfi-14.c \
-    strorder.c strports.c struct.c symbols.c threads.c throw.c values.c	    \
-    variable.c vectors.c version.c vports.c weaks.c
+    pthread-threads.c random.c rdelim.c read.c root.c rw.c scmsigs.c        \
+    script.c simpos.c smob.c sort.c srcprop.c stackchk.c stacks.c stime.c   \
+    strings.c srfi-13.c srfi-14.c strorder.c strports.c struct.c symbols.c  \
+    threads.c threads-plugin.c throw.c values.c variable.c vectors.c        \
+    version.c vports.c weaks.c
 
 DOT_X_FILES = alist.x arbiters.x async.x backtrace.x boolean.x chars.x	 \
     continuations.x debug.x deprecation.x deprecated.x discouraged.x	 \
@@ -208,12 +209,10 @@
 # and people feel like maintaining them.  For now, this is not the case.
 noinst_SCRIPTS = guile-doc-snarf guile-snarf-docs guile-func-name-check
 
-EXTRA_DIST = ChangeLog-gh ChangeLog-scm ChangeLog-threads		\
-    ChangeLog-1996-1999 ChangeLog-2000 cpp_signal.c			\
-    cpp_errno.c cpp_err_symbols.in cpp_err_symbols.c		\
-    cpp_sig_symbols.c cpp_sig_symbols.in cpp_cnvt.awk	\
-    c-tokenize.lex threads-plugin.c version.h.in pthread-threads.c \
-    scmconfig.h.top gettext.h
+EXTRA_DIST = ChangeLog-gh ChangeLog-scm ChangeLog-threads ChangeLog-1996-1999 \
+    ChangeLog-2000 cpp_signal.c	cpp_errno.c cpp_err_symbols.in                \
+    cpp_err_symbols.c cpp_sig_symbols.c cpp_sig_symbols.in cpp_cnvt.awk	      \
+    c-tokenize.lex version.h.in scmconfig.h.top gettext.h
 #    $(DOT_DOC_FILES) $(EXTRA_DOT_DOC_FILES) \
 #    guile-procedures.txt guile.texi
 
Index: guile-core/libguile/pthread-threads.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/pthread-threads.c,v
retrieving revision 1.9
diff -a -u -r1.9 pthread-threads.c
--- guile-core/libguile/pthread-threads.c	5 Apr 2003 19:10:22 -0000	1.9
+++ guile-core/libguile/pthread-threads.c	7 Nov 2004 02:43:39 -0000
@@ -22,7 +22,9 @@
 #  include <config.h>
 #endif
 
-#include "libguile/scmconfig.h"
+#include "pthread-threads.h"
+#include "scmconfig.h"
+#include "threads-plugin.h"
 
 /* Should go to threads-plugin */
 scm_t_mutexattr scm_i_plugin_mutex;
Index: guile-core/libguile/pthread-threads.h
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/pthread-threads.h,v
retrieving revision 1.9
diff -a -u -r1.9 pthread-threads.h
--- guile-core/libguile/pthread-threads.h	5 Apr 2003 19:10:22 -0000	1.9
+++ guile-core/libguile/pthread-threads.h	7 Nov 2004 02:43:40 -0000
@@ -46,6 +46,15 @@
 #define scm_i_plugin_thread_detach	pthread_detach 
 #define scm_i_plugin_thread_self	pthread_self
 #define scm_i_plugin_thread_yield	sched_yield
+#define scm_i_plugin_thread_equal       pthread_equal
+
+/* N.B.: pthread_cleanup_push and _pop are macros! */
+#define scm_i_plugin_thread_cancel      pthread_cancel
+#define scm_i_plugin_thread_cleanup_push pthread_cleanup_push
+#define scm_i_plugin_thread_cleanup_pop  pthread_cleanup_pop
+#define scm_i_plugin_thread_setcancelstate	pthread_setcancelstate
+#define SCM_THREAD_CANCEL_ENABLE	PTHREAD_CANCEL_ENABLE
+#define SCM_THREAD_CANCEL_DISABLE	PTHREAD_CANCEL_DISABLE
 
 extern scm_t_mutexattr scm_i_plugin_mutex; /* The "fast" mutex. */
 
Index: guile-core/libguile/threads-plugin.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/threads-plugin.c,v
retrieving revision 1.4
diff -a -u -r1.4 threads-plugin.c
--- guile-core/libguile/threads-plugin.c	5 Apr 2003 19:10:22 -0000	1.4
+++ guile-core/libguile/threads-plugin.c	7 Nov 2004 02:43:40 -0000
@@ -22,6 +22,12 @@
 #  include <config.h>
 #endif
 
+#include <errno.h>
+#include <stdio.h>
+
+#include "pthread-threads.h"
+#include "threads.h"
+
 int scm_i_plugin_mutex_size = 0;
 int (*scm_i_plugin_mutex_init) (scm_t_mutex *, const scm_t_mutexattr *);
 int (*scm_i_plugin_mutex_lock) (scm_t_mutex *);
Index: guile-core/libguile/threads-plugin.h
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/threads-plugin.h,v
retrieving revision 1.5
diff -a -u -r1.5 threads-plugin.h
--- guile-core/libguile/threads-plugin.h	27 Apr 2004 22:59:04 -0000	1.5
+++ guile-core/libguile/threads-plugin.h	7 Nov 2004 02:43:40 -0000
@@ -21,7 +21,8 @@
  */
 
 \f
-#include <pthread.h> /* This file should *not* need to include pthread.h */
+
+#include "scmconfig.h"
 
 /* Size is checked in scm_init_threads_plugin.
    For reference, sizes encountered include,
@@ -59,6 +60,7 @@
 extern scm_t_rec_mutex_trylock scm_i_plugin_rec_mutex_trylock;
 extern scm_t_rec_mutex_unlock scm_i_plugin_rec_mutex_unlock;
 
+
 /*fixme*/
 #define scm_t_cond			pthread_cond_t
 
Index: guile-core/libguile/threads.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/threads.c,v
retrieving revision 1.68
diff -a -u -r1.68 threads.c
--- guile-core/libguile/threads.c	22 Sep 2004 17:41:37 -0000	1.68
+++ guile-core/libguile/threads.c	7 Nov 2004 02:43:43 -0000
@@ -33,6 +33,8 @@
 #include <sys/time.h>
 #endif
 
+#include <signal.h>
+
 #include "libguile/validate.h"
 #include "libguile/root.h"
 #include "libguile/eval.h"
@@ -114,6 +116,7 @@
   scm_root_state *root;
   SCM handle;
   scm_t_thread thread;
+  SCM cleanup_handlers;
   SCM result;
   int exited;
 
@@ -133,6 +136,7 @@
   t = SCM_THREAD_DATA (z);
   t->handle = z;
   t->result = creation_protects;
+  t->cleanup_handlers = SCM_EOL;
   t->base = NULL;
   scm_i_plugin_cond_init (&t->sleep_cond, 0);
   scm_i_plugin_mutex_init (&t->heap_mutex, &scm_i_plugin_mutex);
@@ -156,6 +160,7 @@
 {
   scm_thread *t = SCM_THREAD_DATA (obj);
   scm_gc_mark (t->result);
+  scm_gc_mark (t->cleanup_handlers);
   return t->root->handle; /* mark root-state of this thread */
 }
 
@@ -285,6 +290,26 @@
   void *handler_data;
 } launch_data;
 
+static void
+handler_cancellation (scm_thread* thread)
+{
+  while (!scm_is_eq(scm_length(thread->cleanup_handlers), SCM_INUM0)) {
+    thread->result = scm_i_eval
+      (SCM_CAR(thread->cleanup_handlers), scm_current_module());
+    thread->cleanup_handlers = SCM_CDR(thread->cleanup_handlers);
+  }
+
+  scm_i_plugin_mutex_lock (&thread_admin_mutex);
+  scm_i_leave_guile();
+
+  all_threads = scm_delq_x (thread->handle, all_threads);
+  thread->exited = 1;
+  thread_count--;
+  scm_thread_detach (thread->thread);
+  scm_i_plugin_mutex_unlock (&thread_admin_mutex);
+  return;
+}
+
 static SCM
 body_bootstrip (launch_data* data)
 {
@@ -315,11 +340,16 @@
   init_thread_creatant (thread, base); /* must own the heap */
   
   data->rootcont = SCM_BOOL_F;
+
+  scm_i_plugin_thread_cleanup_push
+    ((void (*) (void *)) handler_cancellation, (void *) t);
   t->result =
     scm_internal_cwdr ((scm_t_catch_body) body_bootstrip,
 		       data,
 		       (scm_t_catch_handler) handler_bootstrip,
 		       data, base);
+  scm_i_plugin_thread_cleanup_pop(0);
+
   scm_i_leave_guile (); /* release the heap */
   free (data);
 
@@ -370,7 +400,9 @@
     data->body_data = body_data;
     data->handler = handler;
     data->handler_data = handler_data;
+
     t = SCM_THREAD_DATA (thread);
+
     /* must initialize root state pointer before the thread is linked
        into all_threads */
     t->root = SCM_ROOT_STATE (root);
@@ -471,11 +503,77 @@
       scm_i_enter_guile (c);
     }
   res = t->result;
-  t->result = SCM_BOOL_F;
+  /* t->result = SCM_BOOL_F; */
   return res;
 }
 #undef FUNC_NAME
 
+SCM_DEFINE (scm_cancel_thread, "cancel-thread", 1, 0, 0,
+	    (SCM thread),
+"Force the target @var{thread} to terminate, causing all of its "
+"currently registered cleanup handlers to be called.")
+#define FUNC_NAME s_scm_cancel_thread 
+{
+  scm_thread *t = SCM_THREAD_DATA (thread);
+  if (scm_is_eq(scm_member(t->handle, all_threads), SCM_BOOL_F)) {
+    return SCM_BOOL_F;
+  }
+
+  if (!t->exited)
+    {
+      scm_thread *c = scm_i_leave_guile ();
+      while (!THREAD_INITIALIZED_P (t)) {
+	scm_i_plugin_thread_yield ();
+      }
+      scm_i_enter_guile (c);
+      scm_thread_cancel (t->thread);
+    }
+  return SCM_BOOL_T;
+}
+#undef FUNC_NAME
+
+SCM_DEFINE (scm_push_thread_cleanup, "push-thread-cleanup", 1, 0, 0,
+	    (SCM expr),
+"Add an expression @var{expr} to the front of the list of cleanup "
+"handlers for the current thread. These handlers will be evaluated "
+"in a LIFO manner if the current thread is cancelled by another "
+"Scheme thread or by C code, via scm_c_thread_cancel().")
+#define FUNC_NAME s_scm_push_thread_cleanup
+{
+  scm_thread *t = SCM_CURRENT_THREAD;
+  if (scm_is_eq(scm_member(t->handle, all_threads), SCM_BOOL_F)) {
+    return SCM_BOOL_F;
+  }
+  t->cleanup_handlers = scm_cons(expr, t->cleanup_handlers);
+  return SCM_BOOL_T;
+}
+#undef FUNC_NAME
+
+SCM_DEFINE (scm_pop_thread_cleanup, "pop-thread-cleanup", 0, 1, 0,
+	    (SCM exec),
+"Remove the most recently added cleanup handler expression from the "
+"current thread's queue of cleanup handlers.  If a boolean expression "
+"@var{exec} is provided and is true, the cleanup handler will be "
+"evaluated as it is removed.")
+#define FUNC_NAME s_scm_pop_thread_cleanup
+{
+  scm_thread *t = SCM_CURRENT_THREAD;
+  if (scm_is_eq(scm_member(t->handle, all_threads), SCM_BOOL_F)) {
+    return SCM_BOOL_F;
+  }
+
+  SCM result = SCM_EOL;
+  if (t->cleanup_handlers != SCM_EOL) {
+    SCM expr = SCM_CAR(t->cleanup_handlers);
+    t->cleanup_handlers = SCM_CDR(t->cleanup_handlers);
+    if (scm_is_true(exec)) {
+      result = scm_i_eval(expr, scm_current_module());
+    }
+  }
+  return result;    
+}
+#undef FUNC_NAME
+
 /*** Fair mutexes */
 
 /* We implement our own mutex type since we want them to be 'fair', we
@@ -1098,6 +1196,48 @@
   return tv.tv_sec;
 }
 
+/* Thread cleanup handler pushing and popping functions.  These are
+   the same for all threading libraries, because they operate on
+   Guile's internal representation of the thread, not the threading
+   library's. */
+
+static scm_thread*
+scm_t_thread_to_scm_thread(scm_t_thread* needle) {
+  scm_thread* ret = NULL;
+  SCM all_threads = scm_all_threads();
+  while(!scm_is_eq(all_threads, SCM_EOL)) {
+    SCM single_thread = scm_car(all_threads);
+    if (scm_i_plugin_thread_equal(scm_c_scm2thread(single_thread), *needle)) {
+      ret = SCM_THREAD_DATA(single_thread);
+      break;
+    }
+    all_threads = scm_cdr(all_threads);	
+  }
+  return ret;
+}
+
+SCM 
+scm_internal_thread_cleanup_push(scm_t_thread* thread, SCM expr) 
+{
+  scm_thread* t = scm_t_thread_to_scm_thread(thread);
+  if (t != NULL)
+    t->cleanup_handlers = scm_cons(expr, t->cleanup_handlers);
+  return SCM_EOL;
+}
+
+SCM 
+scm_internal_thread_cleanup_pop(scm_t_thread* thread, int exec)
+{
+  SCM ret = SCM_EOL;
+  scm_thread* t = scm_t_thread_to_scm_thread(thread);
+  if ((t != NULL) && (!scm_is_eq(t->cleanup_handlers, SCM_EOL))) {
+    SCM expr = scm_car(t->cleanup_handlers);
+    t->cleanup_handlers = scm_cdr(t->cleanup_handlers);
+    if (exec) ret = scm_eval(expr, t->root->handle);
+  }
+  return ret;
+}
+
 /*** Misc */
 
 SCM_DEFINE (scm_current_thread, "current-thread", 0, 0, 0,
@@ -1217,9 +1357,9 @@
 scm_t_rec_mutex scm_i_defer_mutex;
 
 #if SCM_USE_PTHREAD_THREADS
-# include "libguile/pthread-threads.c"
+#include "libguile/pthread-threads.h"
 #endif
-#include "libguile/threads-plugin.c"
+#include "libguile/threads-plugin.h"
 
 /*** Initialization */
 
Index: guile-core/libguile/threads.h
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/threads.h,v
retrieving revision 1.38
diff -a -u -r1.38 threads.h
--- guile-core/libguile/threads.h	23 Jul 2004 23:51:58 -0000	1.38
+++ guile-core/libguile/threads.h	7 Nov 2004 02:43:44 -0000
@@ -64,9 +64,9 @@
 SCM_API void scm_init_thread_procs (void);
 
 #if SCM_USE_PTHREAD_THREADS
-# include "libguile/pthread-threads.h"
+#include "libguile/pthread-threads.h"
 #else
-# include "libguile/null-threads.h"
+#include "libguile/null-threads.h"
 #endif
 
 /*----------------------------------------------------------------------*/
@@ -97,6 +97,14 @@
 #define scm_thread_detach	scm_i_plugin_thread_detach
 #define scm_thread_self		scm_i_plugin_thread_self
 #define scm_thread_yield	scm_i_plugin_thread_yield
+#define scm_thread_cancel       scm_i_plugin_thread_cancel
+#define scm_thread_setcancelstate	scm_i_plugin_thread_setcancelstate
+/* N.B.: scm_i_plugin_thread_cleanup_push and _pop are defined,
+   but we don't use them here because of the way certain thread libraries
+   implement them; the ones here use a mechanism built into thread creation
+   from thread.c... */
+#define scm_thread_cleanup_push scm_internal_thread_cleanup_push
+#define scm_thread_cleanup_pop  scm_internal_thread_cleanup_pop
 
 #define scm_mutex_init		scm_i_plugin_mutex_init 
 #define scm_mutex_destroy	scm_i_plugin_mutex_destroy
@@ -165,6 +173,9 @@
 SCM_API unsigned long scm_thread_sleep (unsigned long);
 SCM_API unsigned long scm_thread_usleep (unsigned long);
 
+SCM_API SCM scm_internal_thread_cleanup_push(scm_t_thread*, SCM);
+SCM_API SCM scm_internal_thread_cleanup_pop(scm_t_thread*, int);
+
 /* End of low-level C API */
 /*----------------------------------------------------------------------*/
 
@@ -210,6 +221,9 @@
 SCM_API SCM scm_call_with_new_thread (SCM thunk, SCM handler);
 SCM_API SCM scm_yield (void);
 SCM_API SCM scm_join_thread (SCM t);
+SCM_API SCM scm_cancel_thread (SCM t);
+SCM_API SCM scm_push_thread_cleanup (SCM expr);
+SCM_API SCM scm_pop_thread_cleanup (SCM exec);
 SCM_API SCM scm_make_mutex (void);
 SCM_API SCM scm_make_fair_mutex (void);
 SCM_API SCM scm_lock_mutex (SCM m);

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: thread-cancellation-test.scm --]
[-- Type: text/x-scheme; name="thread-cancellation-test.scm", Size: 1321 bytes --]

;; Simple test of thread cancellation.  One thread does something, a second
;; joins on it, and a third cancels the first after you hit enter.  If anyone
;; can figure out why evaluating 'a as part of the cleanup of the sleep-proc
;; thread makes Guile crash, by all means let me know

(use-modules (ice-9 threads))
(define loop-proc 
  (lambda (i) 
    (begin
      ((display "Entering thread...")
       (newline)
       (newline)
       (push-thread-cleanup i)
       (display "counting thread: the counter is ")
       (display i)
       (newline)
       (yield)
       (sleep 2)
       (loop-proc (+ i 1))))))

(define sleep-proc
  (lambda ()
    (begin
      ((display "Entering thread...")
       (push-thread-cleanup 'a)
       (sleep 3000)))))

(define joiner
  (lambda (thread-to-join)
    (let ((result (join-thread thread-to-join)))
      (begin 
	(display "joining thread: the counter was ")
	(display result)
	(newline)))))

;; Redefine cancelled thread as (make-thread loop-proc 1) to see some different
;; behavior

(let ((cancelled-thread (make-thread sleep-proc)))
  (begin
    (make-thread joiner cancelled-thread)
    (read-char (current-input-port))
    (display "cancelling thread...")
    (newline)
    (cancel-thread cancelled-thread)
    (newline)))

[-- Attachment #4: Type: text/plain, Size: 143 bytes --]

_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: hacking on 1.7 threads

[Marius Vollmer]

Julian Graham <joolean@gmail.com> writes:

>   I'm attaching a patch (against HEAD, created in guile/ via 'cvs diff
> -Nau') that represents the current state of my work on thread
> cancellation (except that I removed the cancellation-disabling stuff
> I'd added temporarily to gc.c; I wasn't super confident that it had
> any effect).

Thanks!

I haven't really looked at your code yet and I'm not yet really fully
into the threading stuff, but one thought might be worth sharing
anyway:

Guile uses pthreads to implement multi-threading, but a libguile-using
program must nevertheless obey special rules and can not just use
pthread functions while it is in 'Guile Mode'.  For example, you can
not just use pthread_mutex_lock to lock an arbitrary pthread mutex
because Guile must know when threads are blocking.  You must use
scm_mutex_lock (together with a Guile mutex) or you must leave Guile
Mode.

The same rule is likely advantageous for pthread_cancel.  We can just
state that a thread that is in Guile Mode can not be canceled with
pthread_cancel, but that you must use some other function.

Actually, wouldn't it be enough to just send a thread an asynchronous
exception, like:

    (use-modules (ice-9 threads))

    (define t
      (begin-thread
       (let loop ()
         (pk 'foo)
         (sleep 1)
         (loop))))

    (sleep 2)
    (pk 'bar)
    (system-async-mark quit t)
    (sleep 2)
    (pk 'exit)


Work needs to likely be done so that system-asyncs are able to
interrupt a blocking thread in all situations.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel