RE: Race condition in JV_HASH_SYNCHRONIZATION in libgcj??

["Boehm, Hans" <hans dot boehm at hp dot com>]

> > This is on a MIPS system?  
> Yes.  It is a  SiByte SB1 CPU, which is a dual core mips64 device.
Unfortunately, I'm not really up on what these do with memory
consistency.  MIPS hasn't been very acticely involved in the discussions
we've been having about the C++ memory consistency model.  But I think
that's changing, so I may know more shortly.

> 
> > The failure symptom is that the final value is
> > one or two too low?
> The failure is what seems to be a deadlock.  Normally the 
> test completes 
> in about 10 seconds.  Occasionally however it seems never to complete.
> 
> If I attach gdb, I can see that there are several threads in 
> wait_unnlocked().
Posting the stack traces for all the threads might help.  Or is there a
bug report I should be looking at?

> 
> >   You can't get it to fail with assertions enabled
> > here?  Or it fails the same way with assertions enabled?
> >
> > This code was originally tested on Itanium and X86 
> multiprocessors, so
> > it seems unlikely, but not impossible, that it's generically broken
> > across architectures.  I would normally be suspicious about memory
> > ordering issues.  But MIPS is normally either sequentially 
> consistent
> > or at least close.
> >
> I am likewise suspicious about memory ordering.  I think that perhaps 
> adding memory barriers at the appropriate places might fix it.
> 
> Really I have been thinking that there should be no volatile 
> variables 
> here.  Instead memory barriers should be used at all 
> synchronization points.
Please don't do that.

The right way to do this eventually is with C++ atomic objects.
Unfortunately, those aren't even in the C++ working paper, much less an
official standard, or supported by gcc.  But the current proposal
(http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2393.html) is
headed in a different direction, which many of us think is significantly
better, at least for new code.  Not entirely coincidentally, it bears
some similarities to the aromic_ops package, which is also used in gc7.
But they've diverged recently, primarily because a lot of additional
thought and negotiation has gone into the standards proposal.

The above proposal points to some of the justifications for the approach
chose there.  Using ordinary memory references with fences is
fundamentally on really thin ice, in my opinion much more so than
volatiles (also with fences).  Some common combinations seem to work if
the compiler performs only standard optimizations.  But any compiler is
perfectly entitled to break such code at any time.  The relevant
standards clearly allow the compiler to assume that ordinary variables
do not change asynchronously.  You're violating a critical assumption
made by optimizers; the results can be strange.  Even if you restrict
your attention purely to gcc, I suspect this is a serious issue.
(You're also making the job needlessly difficult for someone reading the
code, by not annotating references that may race.)

(I know the Linux kernel is using more ordinary memory references with
fences.  I think they're living very dangerously.  Not that they have
great alternatives at the moment.)

Of course, volatiles do not have consistent fence semantics.  Thus, even
in the presence of volatiles, we should make sure that there are enough
fences or ordering constraints.  Something like the C++ proposal makes
this much easier.

Hans
> 
> 
> > It might be worth making sure that the compiler isn't moving
> > memory references across a compare_and_swap.
> >
> This could be the problem also.  I have completely re-written all the 
> atomic memory primitives for MIPS and it is possible that I made a 
> mistake.  I should note that the code I replaced had bugs.  The 
> compare_and_swap was returning the old value instead of the result of 
> the compare.
> 
> I will keep working on this.
> 
> Thanks,
> David Daney.
> 
> 
> 
> > Hans
> 
>