RFR: 8025834: NPE in Parallel Scavenge with -XX:+CheckUnhandledOops

[Coleen Phillimore]

Erik,
Thank you for doing the extra cleanup for this.  Did you run the 
nsk.jvmti.testlist tests too though?   These things have a nasty way of 
interacting.   The code looks good though.

thanks,
Coleen

On 10/16/2013 12:09 PM, Erik Helin wrote:
> Hi all,
>
> this patch fixes an issue where an oop in JvmtiBreakpoint,
> JvmtiBreakpoint::_class_loader, was found by the unhandled oop detector.
>
> Instead of registering the oop as an unhandled oop, which would have
> worked, I decided to wrap the oop in a handle as long as it is on the
> stack.
>
> A JvmtiBreakpoint is created on the stack by the two methods
> JvmtiEnv::SetBreakpoint and JvmtiEnv::ClearBreakpoint. This
> JvmtiBreakpoint is only created to carry the Method*, jlocation and oop
> to a VM operation, VM_ChangeBreakpoints. VM_ChangeBreakpoints will, when
> at a safepoint, allocate a new JvmtiBreakpoint on the native heap, copy
> the values from the stack allocated JvmtiBreakpoint and then place/clear the
> newly alloacted JvmtiBreakpoint in
> JvmtiCurrentBreakpoints::_jvmti_breakpoints.
>
> I have updated to the code to check that the public constructor is only
> used to allocate JvmtiBreakpoints on the stack (to warn a future
> programmer if he/she decides to allocate one on the heap). The
> class_loader oop is now wrapped in a Handle for stack allocated
> JvmtiBreakpoints.
>
> Due to the stack allocated JvmtiBreakpoint having the oop in a handle,
> the oops_do method of VM_ChangeBreakpoints can be removed. This also
> makes the oop in the handle safe for use after the VM_ChangeBreakpoint
> operation is finished.
>
> The unhandled oop in the JvmtiBreakpoint allocated on the heap will be
> visited by the GC via jvmtiExport::oops_do ->
> JvmtiCurrentBreakpoints::oops_do -> JvmtiBreakpoints::oops_do ->
> GrowableCache::oops_do -> JvmtiBreakpoint::oops_do, since it is being
> added to.
>
> I've also removed some dead code to simplify the change:
> - GrowableCache::insert
> - JvmtiBreakpoint::copy
> - JvmtiBreakpoint::lessThan
> - GrowableElement::lessThan
>
> Finally, I also formatted JvmtiEnv::ClearBreakpoint and
> Jvmti::SetBreakpoint exactly the same to highlight that they share all
> code except one line. Unfortunately, I was not able to remove this code
> duplication in a good way.
>
> Webrev:
> http://cr.openjdk.java.net/~ehelin/8025834/webrev.00/
>
> Testing:
> - JPRT
> - The four tests that were failing are now passing
>
> Thanks,
> Erik