mirror/guile
mirror/guile
1
Fork 0
mirror of https://git.savannah.gnu.org/git/guile.git synced 2025-05-20 11:40:18 +02:00
Code Activity

Remove scm_enter_guile ()' and scm_leave_guile ()'.

Browse source 
* libguile/threads.c (scm_t_guile_ticket): Remove type.
  (resume, scm_enter_guile, suspend, scm_leave_guile): Remove.
  (scm_i_init_thread_for_guile): Set `t->top' to NULL, which has the
  same effect as calling `scm_enter_guile ()'.
  (scm_leave_guile_cleanup, scm_i_with_guile_and_parent): Remove
  `scm_leave_guile ()' call.
This commit is contained in:
Ludovic Courtès 2009-09-11 00:03:03 +02:00
parent 87e0037091
commit 45f15cac1f
1 changed files with 2 additions and 133 deletions
Download patch file Download diff file Expand all files Collapse all files

135
libguile/threads.c
View file

@ -279,141 +279,12 @@ unblock_from_queue (SCM queue)
return thread;
}
/* Getting into and out of guile mode.
*/
/* Ken Raeburn observes that the implementation of suspend and resume
(and the things that build on top of them) are very likely not
correct (see below). We will need fix this eventually, and that's
why scm_leave_guile/scm_enter_guile are not exported in the API.
Ken writes:
Consider this sequence:
Function foo, called in Guile mode, calls suspend (maybe indirectly
through scm_leave_guile), which does this:
// record top of stack for the GC
t->top = SCM_STACK_PTR (&t); // just takes address of automatic
var 't'
// save registers.
SCM_FLUSH_REGISTER_WINDOWS; // sparc only
SCM_I_SETJMP (t->regs); // here's most of the magic
... and returns.
Function foo has a SCM value X, a handle on a non-immediate object, in
a caller-saved register R, and it's the only reference to the object
currently.
The compiler wants to use R in suspend, so it pushes the current
value, X, into a stack slot which will be reloaded on exit from
suspend; then it loads stuff into R and goes about its business. The
setjmp call saves (some of) the current registers, including R, which
no longer contains X. (This isn't a problem for a normal
setjmp/longjmp situation, where longjmp would be called before
setjmp's caller returns; the old value for X would be loaded back from
the stack after the longjmp, before the function returned.)
So, suspend returns, loading X back into R (and invalidating the jump
buffer) in the process. The caller foo then goes off and calls a
bunch of other functions out of Guile mode, occasionally storing X on
the stack again, but, say, much deeper on the stack than suspend's
stack frame went, and the stack slot where suspend had written X has
long since been overwritten with other values.
Okay, nothing actively broken so far. Now, let garbage collection
run, triggered by another thread.
The thread calling foo is out of Guile mode at the time, so the
garbage collector just scans a range of stack addresses. Too bad that
X isn't stored there. So the pointed-to storage goes onto the free
list, and I think you can see where things go from there.
Is there anything I'm missing that'll prevent this scenario from
happening? I mean, aside from, "well, suspend and scm_leave_guile
don't have many local variables, so they probably won't need to save
any registers on most systems, so we hope everything will wind up in
the jump buffer and we'll just get away with it"?
(And, going the other direction, if scm_leave_guile and suspend push
the stack pointer over onto a new page, and foo doesn't make further
function calls and thus the stack pointer no longer includes that
page, are we guaranteed that the kernel cannot release the now-unused
stack page that contains the top-of-stack pointer we just saved? I
don't know if any OS actually does that. If it does, we could get
faults in garbage collection.)
I don't think scm_without_guile has to have this problem, as it gets
more control over the stack handling -- but it should call setjmp
itself. I'd probably try something like:
// record top of stack for the GC
t->top = SCM_STACK_PTR (&t);
// save registers.
SCM_FLUSH_REGISTER_WINDOWS;
SCM_I_SETJMP (t->regs);
res = func(data);
scm_enter_guile (t);
... though even that's making some assumptions about the stack
ordering of local variables versus caller-saved registers.
For something like scm_leave_guile to work, I don't think it can just
rely on invalidated jump buffers. A valid jump buffer, and a handle
on the stack state at the point when the jump buffer was initialized,
together, would work fine, but I think then we're talking about macros
invoking setjmp in the caller's stack frame, and requiring that the
caller of scm_leave_guile also call scm_enter_guile before returning,
kind of like pthread_cleanup_push/pop calls that have to be paired up
in a function. (In fact, the pthread ones have to be paired up
syntactically, as if they might expand to a compound statement
incorporating the user's code, and invoking a compiler's
exception-handling primitives. Which might be something to think
about for cases where Guile is used with C++ exceptions or
pthread_cancel.)
*/
scm_i_pthread_key_t scm_i_thread_key;
static void
resume (scm_i_thread *t)
{
t->top = NULL;
}
typedef void* scm_t_guile_ticket;
static void
scm_enter_guile (scm_t_guile_ticket ticket)
{
scm_i_thread *t = (scm_i_thread *)ticket;
if (t)
{
resume (t);
}
}
static scm_i_thread *
suspend (void)
{
scm_i_thread *t = SCM_I_CURRENT_THREAD;
/* record top of stack for the GC */
t->top = SCM_STACK_PTR (&t);
/* save registers. */
SCM_FLUSH_REGISTER_WINDOWS;
SCM_I_SETJMP (t->regs);
return t;
}
static scm_t_guile_ticket
scm_leave_guile ()
{
scm_i_thread *t = suspend ();
return (scm_t_guile_ticket) t;
}
static scm_i_pthread_mutex_t thread_admin_mutex = SCM_I_PTHREAD_MUTEX_INITIALIZER;
static scm_i_thread *all_threads = NULL;
@ -699,7 +570,7 @@ scm_i_init_thread_for_guile (SCM_STACKITEM *base, SCM parent)
t->base = base;
#endif
scm_enter_guile ((scm_t_guile_ticket) t);
t->top = NULL;
return 1;
}
else
@ -806,7 +677,6 @@ scm_with_guile (void *(*func)(void *), void *data)
SCM_UNUSED static void
scm_leave_guile_cleanup (void *x)
{
scm_leave_guile ();
on_thread_exit (SCM_I_CURRENT_THREAD);
}
@ -823,7 +693,6 @@ scm_i_with_guile_and_parent (void *(*func)(void *), void *data, SCM parent)
scm_i_pthread_cleanup_push (scm_leave_guile_cleanup, NULL);
res = scm_c_with_continuation_barrier (func, data);
scm_i_pthread_cleanup_pop (0);
scm_leave_guile ();
}
else
res = scm_c_with_continuation_barrier (func, data);