* libguile/threads.c (do_thread_exit_trampoline, on_thread_exit):
(scm_i_init_thread_for_guile): Only register and unregister threads
with bdw-gc when we are building with threads support. Thanks to
Marijn for the report.
GC_unregister_my_thread is only supposed to be called from
a thread other than the main thread, so, it should never be
called when the system is compiled with null threads.
* libguile/threads.c (on_thread_exit)[SCM_USE_NULL_THREADS]: don't
call GC_unregister_my_thread
This is not present in earlier versions of BDW-GC
* configure.ac: check for GC_allow_register_threads
* libguile/threads.c (scm_i_init_thread_for_guile): Only call
GC_allow_register_threads if it is present.
* libguile/threads.c (guilify_self_1): Prevent finalizers from running
before SCM_I_CURRENT_THREAD is set.
(do_thread_exit_trampoline): Leave the thread in the registered state.
(on_thread_exit): Always unregister the thread here.
* libguile/init.h:
* libguile/init.c (scm_i_init_guile): Change arg to this internal
function from SCM_STACKITEM* to void*. Actually it's a
struct GC_stack_base*.
* libguile/bdw-gc.h: Don't do pthread redirects, because we don't want
to affect applications' pthread_* bindings.
* libguile/pthread-threads.h (scm_i_pthread_create)
(scm_i_pthread_detach, scm_i_pthread_exit, scm_i_pthread_cancel)
(scm_i_pthread_sigmask): Do pthread redirects here, in this internal
header.
* libguile/threads.h: Remove declaration of internal
scm_i_with_guile_and_parent. Remove declaration of undefined
scm_threads_init_first_thread. Make declaration of internal
scm_threads_prehistory actually internal, and take a void* (actually a
struct GC_stack_base*).
* libguile/threads.c (GC_get_stack_base): Implement a shim if this
function is unavailable, and fold in the implementations of
get_thread_stack_base.
(GC_call_with_stack_base): Actually implement.
(guilify_self_1): Take a GC_stack_base* as an arg.
(scm_i_init_thread_for_guile): Likewise, and set up libgc for
registration of other threads.
(scm_init_guile): Use GC_get_stack_base instead of our own guesswork.
(with_guile_and_parent, scm_i_with_guile_and_parent): Rework to
trampoline through a GC_call_with_stack_base.
(scm_threads_prehistory): Pass the "base" arg on to guilify_self_1.
* libguile/threads.h: Always declare a scm_i_thread_key, for cleanup
purposes, in the BUILDING_LIBGUILE case.
* libguile/threads.c (scm_i_thread_key): Init with a cleanup handler, so
any guile-specific info for a thread can be cleaned up reliably.
(guilify_self_1): Always set the thread key.
(do_thread_exit_trampoline, on_thread_exit): Enter guile-mode for the
guile-mode cleanup handler, and trampoline through a
gc_call_with_stack_base for reasons explained in the code.
(init_thread_key, scm_i_init_thread_for_guile): Always init the key.
(scm_i_with_guile_and_parent): No need for pthread_cancel cleanup
handlers, as the pthread key destructor will take care of that for
us.
(really_launch): Remove needless pthread_exit call with incorrect
comment.
* configure.ac: Check for GC_call_with_gc_active.
* libguile/threads.h (scm_i_thread): Remove "top", as it's not used.
* libguile/threads.c (with_gc_inactive, with_gc_active): Define shims to
GC_do_blocking and GC_call_with_gc_active.
(scm_i_init_thread_for_guile): Don't do thread base adjustment here,
do it in scm_i_with_guile_and_parent. The previous logic would never
be run.
(scm_i_with_guile_and_parent): If we enter Guile mode, leave it too.
Take care of adjusting the thread stack base here too. Also, call
with_gc_active.
(scm_without_guile): Refactor.
* libguile/threads.c:
* libguile/posix.c: Change a couple #if HAVE_FOO to #ifdef HAVE_FOO to
pacify -Wundef. Some vars are defined or not, whereas some are always
defined to 0 or 1. The inconsistency is unfortunate.
The memory leak is trivially reproducible with:
(define m (make-mutex))
(let loop () (lock-mutex m) (unlock-mutex m) (loop))
or similarly with:
(define p (delay (+ 1 2)))
(let loop () (force p) (loop))
since `force' acquires P's mutex.
It could also lead to premature release of a thread waiting in
`fat_mutex_lock' when a former owner's `do_thread_exit' is run.
* libguile/threads.c (fat_mutex_unlock): When `m->level' becomes 0,
remove MUTEX from `t->mutexes'.
(fat_mutex_lock): Update comment above the `t->mutexes' assignment.
(do_thread_exit): Add an assertion making sure that each mutex in
`t->mutexes' is owned by T.
* libguile/debug.h (scm_t_debug_frame): Remove this type, as it was
internal to the old evaluator.
(SCM_EVALFRAME, SCM_APPLYFRAME, SCM_VOIDFRAME, SCM_MACROEXPF)
(SCM_TAILREC, SCM_TRACED_FRAME, SCM_ARGS_READY, SCM_DOVERFLOW)
(SCM_MAX_FRAME_SIZE, SCM_FRAMETYPE)
(SCM_EVALFRAMEP, SCM_APPLYFRAMEP, SCM_VOIDFRAMEP, SCM_MACROEXPFP)
(SCM_TAILRECP, SCM_TRACED_FRAME_P, SCM_ARGS_READY_P, SCM_OVERFLOWP)
(SCM_SET_MACROEXP, SCM_SET_TAILREC, SCM_SET_TRACED_FRAME)
(SCM_SET_ARGSREADY, SCM_SET_OVERFLOW)
(SCM_CLEAR_MACROEXP, SCM_CLEAR_TRACED_FRAME, SCM_CLEAR_ARGSREADY):
Remove macro accessors to scm_t_debug_frame.
(SCM_DEBUGOBJP, SCM_DEBUGOBJ_FRAME, SCM_SET_DEBUGOBJ_FRAME):
(scm_debug_object_p, scm_make_debugobj): Remove debugobj accessors.
(scm_i_unmemoize_expr): Remove unused declaration.
* libguile/debug.c (scm_debug_options): No more max limit on frame
sizes.
(scm_start_stack): Just call out to scm_vm_call_with_new_stack.
(scm_debug_object_p, scm_make_debugobj, scm_init_debug): No more
debugobj smob type.
* libguile/deprecated.h:
* libguile/deprecated.c (scm_i_deprecated_last_debug_frame)
(scm_last_debug_frame): Remove deprecated debug-frame bits.
* libguile/stacks.c (scm_make_stack): Rework this function and its
dependents to only walk VM frames.
(scm_stack_id): Call out to the holder of the VM frame in question,
which should be a VM or a VM continuation, for the stack ID. Currently
this bit is stubbed out.
(scm_last_stack_frame): Removed. It seems this is mainly useful for a
debugger, and we need to rewrite the debugger to work on the Scheme
level.
* test-suite/tests/continuations.test ("continuations"): Remove test for
last-stack-frame.
* libguile/continuations.h (struct scm_t_contregs):
* libguile/continuations.c (scm_make_continuation):
(copy_stack_and_call, scm_i_with_continuation_barrier): No need to
save and restore debug frames.
* libguile/threads.h (scm_i_thread): Don't track debug frames.
(scm_i_last_debug_frame, scm_i_set_last_debug_frame): Remove macro
accessors.
* libguile/threads.c (guilify_self_1): Don't track debug frames.
* libguile/throw.c: No need to track debug frames in a jmpbuf.
* libguile/vm-engine.c (vm_engine, VM_PUSH_DEBUG_FRAMES): Don't push
debug frames.
* libguile/vm.h:
* libguile/vm.c (scm_vm_call_with_new_stack): New function. Currently
stubbed out though.
The intent is to allow compilation with `-Wundef', which in turn should
make it easier to catch erroneous uses of nonexistent macros.
* libguile/__scm.h: Don't assume `BUILDING_LIBGUILE' is defined.
* libguile/conv-uinteger.i.c (SCM_TO_TYPE_PROTO): Remove unneeded CPP
conditional on `TYPE_MIN == 0'.
* libguile/fports.c: Check for the definition of `HAVE_CHSIZE' and
`HAVE_FTRUNCATE', not for their value.
* libguile/ports.c: Likewise.
* libguile/numbers.c (guile_ieee_init): Likewise with `HAVE_DINFINITY'
and `HAVE_DQNAN'.
* test-suite/standalone/test-conversion.c (ieee_init): Likewise.
* libguile/strings.c: Likewise with `SCM_STRING_LENGTH_HISTOGRAM'.
* libguile/strings.h: Likewise.
* libguile/tags.h: Likewise with `HAVE_INTTYPES_H' and `HAVE_STDINT_H'.
* libguile/threads.c: Likewise with `HAVE_PTHREAD_GET_STACKADDR_NP'.
* libguile/vm-engine.c (VM_NAME): Likewise with `VM_CHECK_IP'.
* libguile/gen-scmconfig.c (main): Use "#ifdef HAVE_", not "#if HAVE_".
* libguile/socket.c (scm_setsockopt): Likewise.
In libgc CVS the `GC_do_blocking ()' declaration is now public but it
uses a slightly different signature for its first argument.
* configure.ac: Check for `GC_fn_type'.
* libguile/threads.c (GC_fn_type)[HAVE_GC_DO_BLOCKING &&
!HAVE_GC_FN_TYPE]: New typedef.
(scm_without_guile): Explicitly cast `without_guile_trampoline' to
`GC_fn_type'. This is because the upstream definition currently
looks like `typedef void * (extern *GC_fn_type)(void *);', which
isn't compatible.
The crux of this problem was that the thread doing a throw, and so
checking scm_i_critical_section_level, was different from the thread
that was in a critical section.
* libguile/async.h (scm_i_critical_section_level): Removed, replaced
by per-thread critical_section_level.
(SCM_CRITICAL_SECTION_START, SCM_CRITICAL_SECTION_END): Use
per-thread critical_section_level.
* libguile/continuations.c (scm_dynthrow): Check per-thread
critical_section_level.
* libguile/threads.c (guilify_self_1): Init per-thread
critical_section_level.
(scm_i_critical_section_level): Removed.
* libguile/threads.h (scm_i_thread): New critical_section_level field.
* libguile/throw.c (scm_ithrow): Check per-thread critical_section_level.
* libguile/threads.c (do_thread_exit): Deal with `t->mutexes' being a
weak list.
(fat_mutex_lock): Use weak-car pairs when building up `t->mutexes'.
* test-suite/tests/threads.test ("mutex-ownership")["mutex with owner
not retained (bug #27450)"]: New test.
* 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.
On Aug 5, 2009, at 10:06, Ken Raeburn wrote:
> (1) In scm_pthread_mutex_lock, we leave and re-enter guile mode so
> that we don't block the thread while in guile mode. But we could
> use pthread_mutex_trylock first, and avoid the costs scm_leave_guile
> seems to incur on the Mac. If we can't acquire the lock, it should
> return immediately, and then we can do the expensive, blocking
> version. A quick, hack version of this changed my run time for
> A(3,8) from 17.5s to 14.5s, saving about 17%; sigaltstack and
> sigprocmask are still in the picture, because they're called from
> scm_catch_with_pre_unwind_handler. I'll work up a nicer patch
> later.
Ah, we already had scm_i_pthread_mutex_trylock lying around; that made
things easy.
A second timing test with A(3,9) and this version of the patch (based
on 1.9.1) shows the same improvement.
* libguile/threads.c (scm_pthread_mutex_lock): Try the mutex before
leaving and reentering guile mode.
Problem was that if an application includes both libguile.h and the
system's setjmp.h, and is compiled on IA64, it gets compile errors
because of jmp_buf, setjmp and longjmp being multiply defined.
* libguile/__scm.h (__ia64__): Define scm_i_jmp_buf, SCM_I_SETJMP and
SCM_I_LONGJMP instead of jmp_buf, setjmp and longjmp.
(all other platforms): Map scm_i_jmp_buf, SCM_I_SETJMP and
SCM_I_LONGJMP to jmp_buf, setjmp and longjmp.
* libguile/continuations.c (scm_make_continuation): Use `SCM_I_SETJMP'
instead of `setjmp'.
(copy_stack_and_call): Use `SCM_I_LONJMP' instead of `longjmp'.
(scm_ia64_longjmp): Use type `scm_i_jmp_buf' instead of `jmp_buf'.
* libguile/continuations.h (scm_t_contregs): Use type `scm_i_jmp_buf'
instead of `jmp_buf'.
* libguile/threads.c (suspend): Use `SCM_I_SETJMP' instead of
`setjmp'.
* libguile/threads.h (scm_i_thread): Use type `scm_i_jmp_buf' instead
of `jmp_buf'.
* libguile/throw.c (JBJMPBUF, make_jmpbuf, jmp_buf_and_retval): Use
type `scm_i_jmp_buf' instead of `jmp_buf'.
(scm_c_catch): Use `SCM_I_SETJMP' instead of `setjmp'.
(scm_ithrow): Use `SCM_I_LONGJMP' instead of `longjmp'.
* libguile/threads.c (scm_join_thread_timed): Recheck t->exited before
looping round to call block_self again, in case thread t has now
exited.
* test-suite/tests/threads.test ("don't hang when joined thread
terminates in SCM_TICK"): New test.
* libguile/threads.c (fat_mutex_unlock): Unblock asyncs when breaking
out of loop.
* test-suite/tests/threads.test (asyncs-still-working?): New function,
to test if asyncs are working (i.e. unblocked). Use this throughout
threads.test, in particular before and after the "timed locking
succeeds if mutex unlocked within timeout" test.
For each thread that goes into Guile mode, Guile pushes a cleanup
function, scm_leave_guile_cleanup, whose purpose is to execute
`scm_leave_guile ()' if the thread is terminated while in Guile mode.
The problem is that there are various places - like
scm_pthread_cond_wait, scm_without_guile and scm_std_select - where
the thread temporarily leaves Guile mode (which means unlocking the
heap mutex), and the cleanup function is still in place. Therefore if
the thread is terminated at these places, the cleanup function ends up
trying to unlock a mutex (the heap mutex) which isn't actually locked.
* libguile/threads.h (scm_i_thread): New heap_mutex_locked_by_self field.
* libguile/threads.c (scm_enter_guile): Set heap_mutex_locked_by_self.
(scm_leave_guile): Only unlock if heap_mutex_locked_by_self is 1.
(guilify_self_1): Initialize heap_mutex_locked_by_self.
(scm_i_thread_sleep_for_gc): Remove incorrect use of t->held_mutex
here.
This fixes bug #24009 reported by Martin Pitt.
* libguile/threads.c (guilify_self_1): Check the return value of
pipe(2).
(scm_std_select): Use `full_read ()' instead of `read ()' when reading
from WAKEUP_FD.
* libguile/async.c (scm_i_queue_async_cell): Use `full_write ()' instead
of write(2) when writing to SLEEP_FD.
* libguile/fports.c (fport_flush): Likewise.
* libguile/posix.c (getgroups): Use the return value of getgroups(2) as
NGROUPS.
(scm_nice): Get the return value of nice(2) to make glibc happy.
* libguile/scmsigs.c (take_signal): Use `full_write ()' instead of
write(2).
* NEWS: Update.
* libguile/threads.c (scm_i_init_thread_for_guile): When the thread is
already guilified, update `t->base' so that it corresponds to the new
stack base. Bug report and patch by Linas Vepstas <linasvepstas@gmail.com>.
* test-suite/standalone/Makefile.am (test_scm_with_guile_CFLAGS,
test_scm_with_guile_LDADD): New.
(check_PROGRAMS, TESTS): Add `test-scm-with-guile'.
The `on_thread_exit ()' function allocates memory via libgc. When
called from the context of a pthread key detructor, the thread is
essentially "dead" already and `GC_lookup_thread ()' returns NULL,
which triggers an assertion in libgc's `thread_local_alloc.c'. This
patch arranges so that `on_thread_exit ()' is called from a suitable
context.
* libguile/threads.c (on_thread_exit): Remove now invalid comment
about access to libgc's TLS.
(init_thread_key): Don't pass `on_thread_exit ()' to
`scm_i_pthread_key_create ()'.
(scm_leave_guile_cleanup): Invoke `do_thread_exit ()'.
(really_launch): Invoke `pthread_exit ()'.
* libguile/threads.h (held_mutex): New field.
* libguile/threads.c (enqueue, remqueue, dequeue): Use critical
section to protect access to the queue.
(guilify_self_1): Initialize held_mutex field.
(on_thread_exit): If held_mutex non-null, unlock it.
(fat_mutex_unlock, fat_cond_free, scm_make_condition_variable,
fat_cond_signal, fat_cond_broadcast): Delete now unnecessary uses
of c->lock.
(fat_mutex_unlock): Pass m->lock to block_self() instead of
c->lock; move scm_i_pthread_mutex_unlock(m->lock) call from before
block_self() to after.
(scm_pthread_cond_wait, scm_pthread_cond_timedwait,
scm_i_thread_sleep_for_gc): Set held_mutex before pthread call;
reset it afterwards.
I was seeing a hang in srfi-18.test, when running make check in master,
in the "exception handler installation is thread-safe" test. It wasn't
100% reproducible, so looked like a race.
The problem is that wait-condition-variable is not actually
atomic in the way that it is supposed to be. It unlocks the mutex,
then starts waiting on the cond var. So it is possible for another
thread to lock the same mutex, and signal the cond var, before the
wait-condition-variable thread starts waiting.
In order for wait-condition-variable to be atomic - e.g. in a race
where thread A holds (Scheme-level) mutex M, and calls
(wait-condition-variable C M), and thread B calls (begin (lock-mutex
M) (signal-condition-variable C)) - it needs to call pthread_cond_wait
with the same underlying mutex as is involved in the `lock-mutex'
call. In terms of the threads.c code, this means that it has to use
M->lock, not C->lock.
block_self() used its mutex arg for two purposes: for protecting
access and changes to the wait queue, and for the pthread_cond_wait
call. But it wouldn't work reliably to use M->lock to protect C's
wait queue, because in theory two threads can call
(wait-condition-variable C M1) and (wait-condition-variable C M2)
concurrently, with M1 and M2 different. So we either have to pass
both C->lock and M->lock into block_self(), or use some other mutex to
protect the wait queue. For this patch, I switched to using the
critical section mutex, because that is a global and so easily
available. (If that turns out to be a problem for performance, we
could make each queue structure have its own mutex, but there's no
reason to believe yet that it is a problem, because the critical
section mutex isn't used much overall.)
So then we call block_self() with M->lock, and move where M->lock is
unlocked to after the block_self() call, instead of before.
That solves the first hang, but introduces a new one, when a SRFI-18
thread is terminated (`thread-terminate!') between being launched
(`make-thread') and started (`thread-start!'). The problem now is
that pthread_cond_wait is a cancellation point (see man
pthread_cancel), so the pthread_cond_wait call is one of the few
places where a thread-terminate! call can take effect. If the thread
is cancelled at that point, M->lock ends up still being locked, and
then when do_thread_exit() tries to lock M->lock again, it hangs.
The fix for that is a new `held_mutex' field in scm_i_thread, which is
set to point to the mutex just before a pthread_cond_(timed)wait call,
and set to NULL again afterwards. If on_thread_exit() finds that
held_mutex is non-NULL, it unlocks that mutex.
A detail is that checking and unlocking held_mutex must be done before
on_thread_exit() calls scm_i_ensure_signal_delivery_thread(), because
the innards of scm_i_ensure_signal_delivery_thread() can do another
pthread_cond_wait() call and so overwrite held_mutex. But that's OK,
because it's fine for the mutex check and unlock to happen outside
Guile mode.
Lastly, C->lock is then not needed, so I've removed it.
* libguile/threads.h (held_mutex): New field.
* libguile/threads.c (enqueue, remqueue, dequeue): Use critical
section to protect access to the queue.
(guilify_self_1): Initialize held_mutex field.
(on_thread_exit): If held_mutex non-null, unlock it.
(fat_mutex_unlock, fat_cond_free, scm_make_condition_variable,
fat_cond_signal, fat_cond_broadcast): Delete now unnecessary uses
of c->lock.
(fat_mutex_unlock): Pass m->lock to block_self() instead of
c->lock; move scm_i_pthread_mutex_unlock(m->lock) call from before
block_self() to after.
(scm_pthread_cond_wait, scm_pthread_cond_timedwait,
scm_i_thread_sleep_for_gc): Set held_mutex before pthread call;
reset it afterwards.
I was seeing a hang in srfi-18.test, when running make check in master,
in the "exception handler installation is thread-safe" test. It wasn't
100% reproducible, so looked like a race.
The problem is that wait-condition-variable is not actually
atomic in the way that it is supposed to be. It unlocks the mutex,
then starts waiting on the cond var. So it is possible for another
thread to lock the same mutex, and signal the cond var, before the
wait-condition-variable thread starts waiting.
In order for wait-condition-variable to be atomic - e.g. in a race
where thread A holds (Scheme-level) mutex M, and calls
(wait-condition-variable C M), and thread B calls (begin (lock-mutex
M) (signal-condition-variable C)) - it needs to call pthread_cond_wait
with the same underlying mutex as is involved in the `lock-mutex'
call. In terms of the threads.c code, this means that it has to use
M->lock, not C->lock.
block_self() used its mutex arg for two purposes: for protecting
access and changes to the wait queue, and for the pthread_cond_wait
call. But it wouldn't work reliably to use M->lock to protect C's
wait queue, because in theory two threads can call
(wait-condition-variable C M1) and (wait-condition-variable C M2)
concurrently, with M1 and M2 different. So we either have to pass
both C->lock and M->lock into block_self(), or use some other mutex to
protect the wait queue. For this patch, I switched to using the
critical section mutex, because that is a global and so easily
available. (If that turns out to be a problem for performance, we
could make each queue structure have its own mutex, but there's no
reason to believe yet that it is a problem, because the critical
section mutex isn't used much overall.)
So then we call block_self() with M->lock, and move where M->lock is
unlocked to after the block_self() call, instead of before.
That solves the first hang, but introduces a new one, when a SRFI-18
thread is terminated (`thread-terminate!') between being launched
(`make-thread') and started (`thread-start!'). The problem now is
that pthread_cond_wait is a cancellation point (see man
pthread_cancel), so the pthread_cond_wait call is one of the few
places where a thread-terminate! call can take effect. If the thread
is cancelled at that point, M->lock ends up still being locked, and
then when do_thread_exit() tries to lock M->lock again, it hangs.
The fix for that is a new `held_mutex' field in scm_i_thread, which is
set to point to the mutex just before a pthread_cond_(timed)wait call,
and set to NULL again afterwards. If on_thread_exit() finds that
held_mutex is non-NULL, it unlocks that mutex.
A detail is that checking and unlocking held_mutex must be done before
on_thread_exit() calls scm_i_ensure_signal_delivery_thread(), because
the innards of scm_i_ensure_signal_delivery_thread() can do another
pthread_cond_wait() call and so overwrite held_mutex. But that's OK,
because it's fine for the mutex check and unlock to happen outside
Guile mode.
Lastly, C->lock is then not needed, so I've removed it.
* ice-9/Makefile.am: Don't compile popen.scm, its behaviour at runtime
is not consistent -- seems to miss some GC references? I suspect a bug
in the compiler. In any case without popen.scm being compiled,
continuations.test, r4rs.tes, and r5rs_pitfall.test do pass.
* libguile/threads.h (scm_i_thread):
* libguile/threads.c (thread_mark, guilify_self_2): Add a field for the
thread's vm. Previously I had this as a fluid, but it seems that newly
created threads share their fluid values from the creator thread; as
expected, I guess. In any case one VM should not be active in two
threads.
* libguile/vm.c (scm_the_vm): Change to access the thread-local vm,
instead of accessing a fluid.
(scm_the_vm_fluid): Removed.
* module/system/vm/vm.scm: Removed *the-vm*.
* libguile/threads.c (scm_threads_mark_stacks): Cast `&t->regs' to
`(void *)' rather than `(SCM_STACKITEM *)' to avoid "warning:
dereferencing type-punned pointer will break strict-aliasing rules"
with GCC 4.2.1 on `i386-unknown-freebsd7.0'.
