[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: srfi-18 requirements
From: |
Neil Jerram |
Subject: |
Re: srfi-18 requirements |
Date: |
Wed, 23 Jan 2008 22:46:23 +0000 |
User-agent: |
Gnus/5.110006 (No Gnus v0.6) Emacs/21.4 (gnu/linux) |
"Julian Graham" <address@hidden> writes:
> Hi Neil,
Hi Julian; sorry for taking so long to follow up on this!
>> 2. If two threads are using pthread_cond_wait and pthread_cond_signal
>> to communicate, and using the cond_var itself as a state
>> indication, they have to be certain that the pthread_cond_wait
>> starts before the pthread_cond_signal, otherwise it won't work.
>
> Right -- holding the right mutexes when you signal / broadcast is
> pretty important.
I don't think holding a mutex when you signal/broadcast is enough.
It's still possible for the signal/broadcast to happen before the
other thread starts waiting.
> I've come up with something similar that seems to work decently and
> seems a bit simple. See what you think (apologies for the
> formatting):
>
> static scm_i_pthread_cond_t wake_up_cond;
> static scm_i_pthread_mutex_t wake_up_mutex;
> static int wake_up_flag = 0;
> int scm_i_thread_go_to_sleep;
>
> void
> scm_i_thread_put_to_sleep ()
> {
> if (threads_initialized_p)
> {
> scm_i_thread *t;
>
> scm_leave_guile ();
> scm_i_pthread_mutex_lock (&thread_admin_mutex);
>
> wake_up_flag = 0;
> scm_i_thread_go_to_sleep = 1;
> for (t = all_threads; t; t = t->next_thread)
NB I have an orthogonal concern here (i.e. possibly yet another issue
with the current code!): If a thread that was running in Guile mode
called scm_leave_guile() to do non-guile stuff for a while, and is
still outside Guile mode, shouldn't it have been removed from the
all_threads list when it called scm_leave_guile()?
(But please feel free to ignore this one for now. We already have
enough loose ends in the air!)
> {
> scm_i_pthread_mutex_lock (&t->heap_mutex);
> }
> scm_i_thread_go_to_sleep = 0;
> }
> }
>
> void
> scm_i_thread_wake_up ()
> {
> if (threads_initialized_p)
> {
> scm_i_thread *t;
>
[B]
> scm_i_pthread_mutex_lock (&wake_up_mutex);
> wake_up_flag = 1;
> scm_i_pthread_cond_broadcast (&wake_up_cond);
> scm_i_pthread_mutex_unlock (&wake_up_mutex);
> for (t = all_threads; t; t = t->next_thread)
> {
> scm_i_pthread_mutex_unlock (&t->heap_mutex);
> }
> scm_i_pthread_mutex_unlock (&thread_admin_mutex);
> scm_enter_guile ((scm_t_guile_ticket) SCM_I_CURRENT_THREAD);
> }
> }
>
> void
> scm_i_thread_sleep_for_gc ()
> {
> scm_i_thread *t = suspend ();
>
> scm_i_pthread_cleanup_push ((void (*)(void *)) scm_i_pthread_mutex_unlock,
> &wake_up_mutex);
> scm_i_pthread_mutex_lock (&wake_up_mutex);
> scm_i_pthread_mutex_unlock (&t->heap_mutex);
> do
I think you need to check !wake_up_flag at the start of the loop. I
think it is possible that when the loop starts, the GC thread has
already set wake_up_flag to 1 and signalled wake_up_cond.
> {
> scm_i_pthread_cond_wait (&wake_up_cond, &wake_up_mutex);
> }
> while (!wake_up_flag);
[A]
> scm_i_pthread_mutex_lock (&t->heap_mutex);
> scm_i_pthread_mutex_unlock (&wake_up_mutex);
Do the locks of t->heap_mutex and wake_up_mutex really need to overlap
like this? I think this could lead to a deadlock: at [A] above, the
non-GC thread holds wake_up_mutex and tries to lock t->heap_mutex,
whereas at [B] above, the GC thread holds t->heap_mutex (for every
thread) and tries to lock wake_up_mutex.
If there isn't a hard reason for the overlapping, the two lock/unlock
pairs just above can be swapped, and that eliminates the deadlock
possibility.
Otherwise I think your code looks good.
> Of course -- all I meant by this was that in the existing thread tests
> (and in much of the SRFI-18 test code I wrote) the lifespans of
> threads besides the main thread (and the signal delivery thread) are
> usually short enough that they don't end up participating in this
> whole co-op GC process. Maybe we need some test code for
> longer-running, guile-mode threads. (Perhaps developers with
> multi-threaded Guile application development under their belts would
> care to chime in here?)
Yes, some longer-running threads would be good. Does anyone have some
multi-threaded code that we could use for testing?
> I'm not totally sure -- I'll have to add some more logs and get back
> to you. I think are definitely some places where an extra SCM_TICK
> might do some good (in fat_cond_timedwait, e.g.).
OK, no worries. Let's handle this when we need to.
Regards,
Neil
- Re: srfi-18 requirements, Neil Jerram, 2008/01/01
- Re: srfi-18 requirements, Julian Graham, 2008/01/04
- Re: srfi-18 requirements, Neil Jerram, 2008/01/04
- Re: srfi-18 requirements, Julian Graham, 2008/01/06
- Re: srfi-18 requirements, Neil Jerram, 2008/01/08
- Re: srfi-18 requirements, Julian Graham, 2008/01/10
- Re: srfi-18 requirements, Neil Jerram, 2008/01/16
- Re: srfi-18 requirements, Julian Graham, 2008/01/19
- Re: srfi-18 requirements,
Neil Jerram <=
- Re: srfi-18 requirements, Julian Graham, 2008/01/23
- Re: srfi-18 requirements, Neil Jerram, 2008/01/24
- Re: srfi-18 requirements, Julian Graham, 2008/01/24
- Re: srfi-18 requirements, Julian Graham, 2008/01/27
- Re: srfi-18 requirements, Neil Jerram, 2008/01/08