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

David Holmes david.holmes at oracle.com
Thu Nov 22 16:01:42 PST 2012


Hi Stefan,

Not my area so a minor comment not a review :)

thread.hpp

  481   // GC support
  482   // Apply "f->do_oop" to all root oops in "this".
  483   // Apply "cf->do_code_blob" (if !NULL) to all code blobs active 
in frames
  484   virtual void oops_do(OopClosure* f, CLDToOopClosure* cld_f, 
CodeBlobClosure* cf);

can you add a comment that cld_f is not used/needed by Thread::oops_do

Thanks,
David

On 23/11/2012 12: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


More information about the hotspot-dev mailing list