RFR (XS): 6843375: Debuggee VM crashes performing mark-sweep-compact

[Erik Helin]

Hi Jon,

thanks for reviewing!

On 05/08/2013 08:21 PM, Jon Masamitsu wrote:
> Erik,
>
> Change looks good.
>
> Would you consider changing the name of _breakpoints
>
>> class VM_ChangeBreakpoints : public VM_Operation {
>> private:
>>   JvmtiBreakpoints* _breakpoints;
>
> to jvmtiExport_breakpoints?

Would _jvmti_breakpoints make more sense? The VM_ChangeBreakpoints 
constructor always gets passed 
JvmtiCurrentBreakpoints::_jvmti_breakpoints as argument.

On 05/08/2013 08:21 PM, Jon Masamitsu wrote:
> I'm looking for a way to alert
> the reader of the code that this list of breakpoints is part of
> (owned if you will) by some other variable so that the reader
> will not think it is part of the responsibility of VM_ChangeBreakpoints
> to update it (as it was erroneously done before you fix).
> Does that make sense?  Is the name change enough?  I was
> trying to come up with some refactoring of the code to make
> it more obvious that _breakpoints should not be processes
> by VM_ChangeBreakpoints but this is the best I could do.

Another suggestion would be to remove the JvmtiBreakpoints* arguments to 
VM_ChangeBreakpoints and instead directly call 
JvmtiCurrentBreakpoints::get_jvmti_breakpoints to show the reader of the 
code that VM_ChangeBreakpoints always make use of 
JvmtiCurrentBreakpoints::_jvmti_breakpoints.

That way, one would (hopefully) see the JvmtiExport::oops_do calls 
JvmtiCurrentBreakpoints::oops_do and realize that it is not 
VM_ChangeBreakpoints responsibility to call oops_do.

What do you think?

Thanks,
Erik

> This is a throw away suggestion if you like.
>
> Jon
>
>
>
> On 5/8/2013 8:51 AM, Erik Helin wrote:
>> Hi all,
>>
>> this change fixes a problem when a VM_ChangeBreakpoint operation is
>> enqueued on the VMOperationQueue while at the same time as a GC is
>> running. The GC will process the strong roots and will call both
>> VMOperationQueue::oops_do (via Threads::oops_do) and
>> JvmtiExport::oops_do.
>>
>> VMOperationQueue::oops_do will call VM_ChangeBreakpoint::oops_do which
>> calls JvmtiBreakpoints::oops_do. The problem is that
>> JvmtiExport::oops_do calls JvmtiCurrentBreakpoints::oops_do which
>> points to the same JvmtiBreakpoints as the one VM_ChangeBreakpoint
>> just called oops_do on.
>>
>> This change remove the code VM_ChangeBreakpoint that calls
>> JvmtiBreakpoints::oops_do. All the GCs will visit the breakpoints via
>> JvmtiExport.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~ehelin/6843375/webrev/
>>
>> Testing:
>> JPRT
>>
>> Bug:
>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6843375
>>
>> Thanks,
>> Erik
>