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
