RFR(S): 7147724: G1: hang in SurrogateLockerThread::manipulatePLL

[John Coomes]

John Cuthbertson (john.cuthbertson at oracle.com) wrote:
> Hi Bengt,
> 
> Thanks for looking again. Replies inline....
> 
> On 03/09/12 02:25, Bengt Rutisson wrote:
> > ...
> > I understand that we want to use Mutex::_no_safepoint_check_flag since 
> > it is defined. But since there is no Mutex::_safepoint_check_flag or 
> > similar I think we break the abstraction by doing 
> > "safepoint_check_flag = !Mutex::_no_safepoint_check_flag". This kind 
> > of exposes that we are dealing with a bool. So, given this I think I 
> > would prefer to change the code to just:
> >
> > safepoint_check_flag = Thread::current()->is_Java_thread();
> 
> OK. Done, but with a small difference. For Java threads we wish to pass 
> false so that we don't skip the safepoint check so I used "bool 
> no_safepoint_check_flag = !Thread::current()->is_Java_thread();" and can 
> then just pass in the value.
> 
> New webrev: http://cr.openjdk.java.net/~johnc/7147724/webrev.2/

I looked at the first version, then let you and Bengt discuss it a bit
before looking again :-).  This version looks good to me.

One nit in g1CollectedHeap.cpp:

 999     if (result != NULL ) {

you added a missing space before '{'.  If you're going to touch that
line, could you also delete the extra space before ')'?

-John