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

[Boehm, Hans]

Sorry about the long delay.

I don't quite understand the problem here.  If GC_stop_count has
just been incremented, then I'm about to send another suspend
signal to the thread, and it will have to stop again before
we think the world is stopped.  

Can you be a bit more specific about the race here?

Thanks.

Hans

> -----Original Message-----
> From: Ben Maurer [mailto:bmaurer at ximian.com] 
> Sent: Tuesday, July 12, 2005 11:29 PM
> To: Boehm, Hans
> Cc: gc at napali.hpl.hp.com; mono-devel-list at lists.ximian.com
> Subject: RE: [Gc] [PATCH] Race condition when restarting threads
> 
> 
> On Tue, 2005-07-12 at 11:42 -0700, Boehm, Hans wrote:
> > > Your patch had the fields set as volatile, so shouldn't the
> > > compiler ensure that the cpu does not reorder the stores?
> > We had a long discussion of that on the C++ memory model list. The 
> > answer is architecture dependent.  Volatile will generally prevent 
> > compiler reordering.  It usually introduces the necessary hardware 
> > barriers on Itanium, but not, for example, on PowerPC.
> 
> I think your version has a race if this is the case:
> 
> > +    sigsuspend(&suspend_handler_mask);        /* Wait for signal */
> > +    while (GC_world_is_stopped && GC_stop_count == my_stop_count) {
> 
> Imagine that this thread gets a spurious signal. The 
> GC_stop_count++ statement has already taken effect, but the 
> GC_world_is_stopped = TRUE has not, the thread would bypass 
> the wait, causing the world not to be stopped.
> 
> In fact, how do we know that my_stop_count is correct?
> 
> When I put this in the mono tree, I'd really have something 
> with barriers in it and use the version that I suggested. 
> Without the barriers, am a bit worried about the correctness 
> (especially since the issues happen on platforms I am not 
> testing with).
> 
> -- Ben
> 
>