CRR: 7034139: G1: assert(Thread::current()->is_ConcurrentGC_thread()) failed: only a conc GC thread can call this (S)

[Bengt Rutisson]

Tony,

I think this fix looks good.

I like that you have a way of testing this, but if it is possible I 
would like to see if it can be made more obvious in the code what parts 
are "real" code and what parts are there just for testing.

For example, I think I would prefer that concurrentMark.cpp had calls like:

DEBUG_ONLY(force_overflow()->update());

rather than having this in concurrentMark.hpp:

void update() PRODUCT_RETURN;

That way someone browsing the code in concurrentMark.cpp can quickly see 
the difference between test code and product code. I realize that this 
might be a matter of taste, so I'm ok with the way it is now as well.

Bengt

On 2011-04-23 00:58, Tony Printezis wrote:
> Thanks to John Cuthbertson for looking at this. I took his advice and 
> I'm going to disable the forced overflow by default (by setting the 
> default parameter to 0), but leave the code in as it's helpful. Latest 
> version here:
>
> http://cr.openjdk.java.net/~tonyp/7034139/webrev.2/
>
> Tony
>
> Tony Printezis wrote:
>> Hi all,
>>
>> I'd still like a couple of code reviews for this. Here's the latest 
>> version (I only rephrased a couple of comments, so if you're looking 
>> at the earlier version already you can ignore this one):
>>
>> http://cr.openjdk.java.net/~tonyp/7034139/webrev.1/
>>
>> Tony
>>
>> Tony Printezis wrote:
>>> Hi,
>>>
>>> Could I get a couple of people to look at this? (I'd like to push 
>>> this this week if possible)
>>>
>>> http://cr.openjdk.java.net/~tonyp/7034139/webrev.0/
>>>
>>> The actual fix is reasonably small (leave / join the 
>>> SuspendibleThreadSet only if we are in concurrent mode). Most of the 
>>> changes are new infrastructure to cause a fixed number of overflows 
>>> during marking (in non-product builds of course) to stress the 
>>> overflow code. This was the only way I could reliably reproduce the 
>>> failure. This did uncover a couple of extra issues which I also fixed:
>>>
>>> - If we overflow during remark we should not actually deal with it 
>>> during remark but we should abort the remark pause and restart a 
>>> concurrent mark phase. For some reason we were not doing that. I 
>>> fixed that (for this I had to ensure that the overflow flag is not 
>>> cleared when we exit the do_marking_step() method).
>>> - Because we were clearing the overflow, it was also possible that 
>>> the workers would deadlock (for that to happen a worker had to 
>>> finish handling one overflow and immediately raise another one, so 
>>> it was highly unlikely to occur in prcatice; good to find it and 
>>> eliminate it though).
>>>
>>> I've already tested it, I'll run more tests overnight.
>>>
>>> Tony
>>