[9] RFR (S): 8060147: SIGSEGV in Metadata::mark_on_stack() while marking metadata in ciEnv

[Coleen Phillimore]

Vladimir,

I'm sorry that I didn't get to my email today but from the discussion I 
think changing two occurrences of "class_loader" in 
ciInstanceKlass::ciInstanceKlass to "klass_holder" would have solved 
your problem.

Unless, you can add a ciMethod or ciMethodData without adding a 
ciInstanceKlass (which I don't think you can).

I think Roland pointed out a flaw though that you can safepoint before 
adding a ciInstanceKlass though, which you could fix by moving this up 
in ciMethod::ciMethod to before the safepoint.

   _holder = env->get_instance_klass(h_m()->method_holder());

I know I suggested adding the ciObject in ciMetadata but that's because 
this is done somewhere that is hard to find.  A good comment that this 
is what keeps metadata that ci points to from being unloaded by GC would 
help a lot with that.

Thanks,
Coleen


On 10/30/2014 02:06 PM, Vladimir Kozlov wrote:
> I would go with webrev.01 (updated initial version).
>
> Regards,
> Vladimir
>
> On 10/30/14 7:55 AM, Vladimir Ivanov wrote:
>>>> As a solution, _holder can be passed into ciMethod::ciMethod as a
>>>> parameter. It should fix the problem.
>>>
>>> The first change you suggested
>>> (http://cr.openjdk.java.net/~vlivanov/8060147/webrev.00) would fix the
>>> ciMethod::ciMethod problem, right? The code would be more robust that
>>> way and other similar issues could be avoided.
>> Yes, initial version fixes ciMethod::ciMethod problem. It's also more
>> robust and easier to reason about.
>>
>> The downside is that for every ciMetadata instantiation we do more work.
>>
>> I have an alternative version:
>> http://cr.openjdk.java.net/~vlivanov/8060147/webrev.anon.00/
>>
>> Initial version (updated):
>> http://cr.openjdk.java.net/~vlivanov/8060147/webrev.01/
>>
>> I like initial version more, but I don't have strong opinion here.
>>
>> Best regards,
>> Vladimir Ivanov