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

[Coleen Phillimore]

On 10/29/14, 12:25 PM, Mikael Gerdin wrote:
> Hi Vladimir,
>
> On 2014-10-29 14:57, Vladimir Ivanov wrote:
>> http://cr.openjdk.java.net/~vlivanov/8060147/webrev.00
>> https://bugs.openjdk.java.net/browse/JDK-8060147
>>
>> ciObjectFactory doesn't keep cached Metadata* alive. ciMetadata is
>> different from ciObject - it doesn't store JNI handle, but a raw
>> Metadata* pointer.
>>
>> In order to avoid Metadata* vanishing during compilation, I cache
>> ciObject for it's holder, which should keep corresponding Metadata*
>> alive. Cached objects have the same lifetime as the owning
>> ciObjectFactory (no cache pruning), so Metadata* will be available as
>> long as ciObjectFactory instance is used.
>>
>> Also, cleaned relevant comments and strengthened some asserts (since
>> NULL keys are not allowed).
>>
>> Testing: jprt, stress mode - full Nashorn/Octane with aggressive VM
>> anonymous class unloading (LambdaForm/MethodHandle caching completely
>> turned off).
>
> Have you tried this with G1?
>
> I'm asking because G1's concurrent marking algorithm behaves slightly 
> different to the other gcs, and I'm unsure if you need to also need to 
> call G1SATBCardTableModRefBS::enqueue (like ensure_metadata_alive does).

ensure_metadata_alive doesn't handle MethodData.   Should it?
>
> When implementing class unloading for G1 we saw some similar crashes 
> and I think I redeemed them by calling ensure_metadata_alive for 
> Metadata retrieved from the MDO's (since MDO's are in fact "weak" 
> Metadata references).
>
> Essentially, since we have a compilation executing I thought that the 
> reciever of the methods compiled must all be alive, and if the 
> receivers are alive then they should keep all the holders alive.
>
> ciInstanceKlass::ciInstanceKlass creates a JNIHandle to the 
> class_loader() of an InstanceKlass being wrapped in the CI, perhaps 
> that should be changed to klass_holder() to account for anonymous 
> classes?

This is probably unnecessary now with Vladimir's change and why it 
didn't crash sooner.

Coleen
>
> /Mikael
>
>>
>> Thanks!
>>
>> Best regards,
>> Vladimir Ivanov