null-check in G1KeepAliveClosure::do_oop() - needed?

[Tony Printezis]

Thomas,

Thanks for getting back to me promptly. I'll create a CR and post a patch.

Tony

On 9/8/14, 10:32 AM, Thomas Schatzl wrote:
> Hi,
>
> On Mon, 2014-09-08 at 10:19 -0400, Tony Printezis wrote:
>> Hi all,
>>
>> I was a bit perplexed by the following code:
>>
>> class G1KeepAliveClosure: public OopClosure {
>>     ...
>>     void do_oop(oop* p) {
>>       oop obj = *p;
>>
>>       G1CollectedHeap::in_cset_state_t cset_state = _g1->in_cset_state(obj);
>>       if (obj == NULL || cset_state == G1CollectedHeap::InNeither) {
>>         return;
>>       }
>>
>> Is the NULL check needed (can obj actually be NULL)?
> No, does not seem so. The only use of G1KeepAliveClosure seems to be the
> use in JNIHandleBlock::weak_oops_do. That one already filters out NULL
> values.
>
>> If it is, why is it
>> done after the look-up in in_cset_state()? And is in_cset_state()
>> actually robust wrt obj being NULL?
> No. This is a bug. If you have time, please change the NULL check to an
> assert and remove it in the regular code.
>
> Thanks,
>    Thomas
>
>

-- 
Tony Printezis | JVM/GC Engineer / VM Team | Twitter

@TonyPrintezis
tprintezis at twitter.com