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

[Bengt Rutisson]

Hi John,

On 1/30/13 10:51 PM, John Cuthbertson wrote:
> 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) {
> ..
> }

Yes, much better! :)

>> 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.

OK. I don't have strong opinions about it. Let's leave the title as it is.

Bengt

>
> Thanks,
>
> JohnC
>