Hello fellow Guiler of the Guild!
Andy Wingo <wi...@pobox.com> skribis:
> Hello guild-comrade Courtès!
>
> On Mon 27 Jun 2011 18:00, l...@gnu.org (Ludovic Courtès) writes:
>
>> "Andy Wingo" <wi...@pobox.com> skribis:
>>
>>> commit 3b971a59b55586a236c3621a55515d9272ee5c80
>>> Author: Andy Wingo <wi...@pobox.com>
>>> Date: Thu Jun 23 11:24:16 2011 +0200
>>>
>>> don't spawn the signal delivery thread in a thread-exit handler
>>>
>>> * libguile/threads.c (do_thread_exit): Remove needless spawn of the
>>> signal delivery thread.
>>
>> Out of curiosity, what’s the rationale? Should it be done in stable-2.0
>> as well?
>
> Out of curiosity, I looked :) The commit that added the spawning code
> was 2e77f7202b11ad0003831fcff94ec7db80cca015 from you, which
> unfortunately has no log.
After a moment of shame, I looked at libguile/ChangeLog-2008 and found
evidence that I’m only confederate at worst:
2007-10-20 Julian Graham <jool...@gmail.com>
Add support for thread cancellation and user-defined thread
cleanup handlers. Small rework by Ludovic Courtès.
* null-threads.h (scm_i_pthread_cancel,
scm_i_pthread_cleanup_push, scm_i_pthread_cleanup_pop): New.
* pthread-threads.h (scm_i_pthread_cancel,
scm_i_pthread_cleanup_push, scm_i_pthread_cleanup_pop): New.
* scmsigs.c (scm_i_signal_delivery_thread,
signal_delivery_thread_mutex): New.
(signal_delivery_thread): Leave when `read_without_guile ()'
returns zero.
(start_signal_delivery_thread): Acquire SIGNAL_DELIVERY_THREAD
before spawning the thread. Initialize
SCM_I_SIGNAL_DELIVERY_THREAD.
(ensure_signal_delivery_thread): Renamed to...
(scm_i_ensure_signal_delivery_thread): this.
(scm_i_close_signal_pipe): New.
* scmsigs.h: Updated.
* threads.c (thread_mark): Mark `t->cleanup_handler'.
(guilify_self_1): Initialize `t->cleanup_handler' and
`t->canceled'.
(do_thread_exit): Invoke `t->cleanup_handler'.
(on_thread_exit): Call `scm_i_ensure_signal_delivery_thread ()'.
Call `scm_i_close_signal_pipe ()' when the next-to-last thread
vanishes.
(scm_leave_guile_cleanup): New.
(scm_i_with_guile_and_parent): Use `scm_i_pthread_cleanup_push ()'
and `scm_leave_guile_cleanup ()' to leave guile mode, rather
than call `scm_leave_guile ()' after FUNC.
(scm_cancel_thread, scm_set_thread_cleanup_x,
scm_threads_cleanup): New.
(scm_all_threads): Remove SCM_I_SIGNAL_DELIVERY_THREAD from the
returned list.
* threads.h (scm_i_thread)[cleanup_handler, canceled]: New
fields.
Add declarations of new functions.
Pffew.
The explanation appears to be at
<http://lists.gnu.org/archive/html/guile-devel/2007-10/msg00029.html>:
* If the signal delivery thread got launched a little bit too late, it
could be holding its startup mutex and then attempt to grab the
thread_admin_mutex, which could be held by a thread that was in the
process of being canceled and which was trying to obtain the signal
delivery thread's startup mutex. I've resolved this by forcing the
signal delivery thread to start (if it hasn't already) during thread
cancellation of any thread.
At first sight it still holds.
I checked on ‘stable-2.0’ by applying
3b971a59b55586a236c3621a55515d9272ee5c80 and then running threads.test
under Helgrind, which didn’t report any lock order issue. It could mean
that I was lucky, though.
> Actually, let's take this opportunity to have a moment of silence for
> Arch.
And the terrible tla-cvs-sync!
>> As a side note, why does this area have to change so often? :-)
>
> I understand the frustration :) Threads are hard, signals are hard, GC
> and threads and signals is hard. But I do hope it's getting better.
We have too few tests for the tricky corner cases, but OTOH they are
difficult to write. The number of simpler thread-related tests has
increased though, which probably means we caught a few bugs already, so
there’s hope! ;-)
Thanks,
Ludo’.
