RFR(XS): 8001384: G1: assert(!is_null(v)) failed: narrow oop value can never be zero

[John Cuthbertson]

Hi Bengt,

Thanks for looking ove the change

On 1/30/2013 1:18 PM, Bengt Rutisson wrote:
>
> Hi John,
>
> Nice detective work to find this issue! Based on your very good 
> explanation I think this change looks good!
>
> One small detail. I think the comment in JavaThread::exit() is a 
> little confusing. I think you are right in your explanation (and code 
> change) that the deferred card marks need to be flushed for all 
> collectors. But the comment seems to suggest that it only affects G1:
>
> 1899   // Flush any deferred card marks. Flushing may add cards to this
> 1900   // thread's G1 dirty card queue so we have to do this before
> 1901   // flushing the G1 barrier queues.
>
> I would prefer to move this comment in to the G1 specific section 
> where we flush the barrier queues:
>
> 1906   // We must flush G1-related buffers before removing a thread from
> 1907   // the list of active threads.
> 1908   if (UseG1GC) {
> 1909     flush_barrier_queues();
> 1910   }
>
> The comment could say something like "For G1, flushing the deferred 
> card marks may add cards to the thread's dirty card queue. So we need 
> to flush the barrier queues after we have flushed the deferred card 
> marks.".

Good suggestions. Done. I would say the latter part a bit differently 
since we need to flush G1 barrier queues anyway but now we have an 
ordering imposed upon us:

// We must flush the G1-related buffers before removing a thread
// from the list of active threads. We must do this after any deferred
// card marks have been flushed (above) so that any entries that are
// added to the thread's dirty card queue as a result are not lost.
if (UseG1GC) {
..
}

> Also, should we update the title of the bug? Should it have the "G1:" 
> prefix since it is not a G1 specific fix?

I'm not sure and don't really have a preference. My only concern is that 
the original problem was reported against G1 (and I don't think I've 
seen any issue with the other collectors that could be traced to this). 
Keeping the G1 might make it easier for SQE and others to find this CR 
when triaging other G1 test failures.

Thanks,

JohnC