Review request (M): 8003720: NPG: Method in interpreter stack frame can be deallocated

[Stefan Karlsson]

On 11/26/2012 03:55 PM, Coleen Phillimore wrote:
> Looks fine from the runtime code. 

Thanks.

> Someone from GC will have to review the GC code.

John Coomes has now reviewed this, so I'm going to push this.

thanks,
StefanK

> thanks,
> Coleen
>
> On 11/22/2012 9:14 AM, Stefan Karlsson wrote:
>> http://cr.openjdk.java.net/~stefank/8003720/webrev/
>>
>> Description from CR:
>> In virtual calls the Method pointer in the interpreter stack frame is 
>> not kept alive by anything other than the "this" pointers to that 
>> method. If bytecodes overwrite the "this" pointer, then call a full 
>> GC, the class loader containing the Method* can be unloaded and the 
>> Method* deallocated.
>>
>> This is also a problem with JSR292 MethodHandle static code because 
>> the MethodHandle containing the mirror for the interpreted method 
>> Method* is not on the stack if a GC occurs.
>>
>> Fix proposal:
>> The "obvious" solution to this problem would be to apply the root 
>> scanning OopClosure to the Klass::_java_mirror field of the method in 
>> the interpreted frame. However, doing this might cause us to scan the 
>> same metadata oop location more than once, which is not allowed by 
>> some of the HotSpot GCs. We currently solve similar situations by 
>> always "claiming" and start scanning from the ClassLoaderData and 
>> then proceed down into the Klasses of that class loader.
>>
>> For this bug we do the same. All old collections, where class 
>> unloading can occur, pass down a closure that is applied to the 
>> ClassLoaderData of the Klass of the Method in the interpreted frame. 
>> This closure does the claiming and proceeds to scan the class 
>> metadata. Note that during young collections, where we don't do class 
>> unloading, all classes are already used as strong roots and we don't 
>> have to apply this new closure in the interpreted frame.
>>
>> Testing:
>> The added test was initially written by John Rose. I only ported it 
>> to JTreg and made some artistic cleanups to it.
>>
>> thanks,
>> StefanK
>