[Mono-devel-list] RE: [Gc] [PATCH] Race condition when restarting threads

[Ben Maurer]

On Mon, 2005-07-11 at 14:15 -0700, Boehm, Hans wrote:
> I've attached a different patch, which I think should solve the
> problem without additional synchronization and context switches,
> at least in the vast majority of cases.  (It should solve the
> problem in all cases.  Additional context switches will be
> needed only if the sigsuspend wakes up early, I claim.)
> 
> Please let me know if you have any problems with this, or if
> this doesn't look right to you.  I tested only superficially.

I'll try to test this as soon as I get a chance.

One question, which is probably a case of me not having any clue about
the code:

> -    do {
> -           me->stop_info.signal = 0;
> -           sigsuspend(&suspend_handler_mask);        /* Wait for signal */
> -    } while (me->stop_info.signal != SIG_THR_RESTART);
> +    /* We do not continue until we receive a SIG_THR_RESTART,  */
> +    /* but we do not take that as authoritative.  (We may be   */
> +    /* accidentally restarted by one of the user signals we    */
> +    /* don't block.)  After we receive the signal, we use a    */
> +    /* primitive and expensive mechanism to wait until it's    */
> +    /* really safe to proceed.  Under normal circumstances,    */
> +    /* this code should not be executed.                       */
> +    sigsuspend(&suspend_handler_mask);        /* Wait for signal */
> +    while (GC_world_is_stopped && GC_stop_count == my_stop_count) {
> +        GC_brief_async_signal_safe_sleep();
> +#       if DEBUG_THREADS
> +         GC_err_printf0("Sleeping in signal handler");
> +#       endif
> +    }

Why can't you just say

do {
	sigsuspend (&suspend_handler_mask);
} while (GC_world_is_stopped && GC_stop_count == my_stop_count);

-- Ben