invalid oops during oop_do() iteration?

[Tony Printezis]

Not very familiar with this code, but isn't mark_sweep_phase3 the phase 
that adjusts the reference fields in the live objects to reflect the new 
location of the referent *before* the objects are moved? Does this 
explain Keith's observation?

Y. S. Ramakrishna wrote:
> Can you send a pointer to the changes that you made that led
> to this issue? In my repo, I do not see a method_at() method
> of JvmtiBreakpoint to try to understand what the problem is.
>
> I didn't understand your statement "methopdOops will always be valid and
> refer to the same method". (methodOops that are marked by GC as reachable
> will be kept, but if you stash methodOops in yr breakpoint structure,
> and these are weak pointers, as i assume they want to be, these would
> be cleared (nulled) during a post-mark weak references iteration, after
> which class unloading is also done.)
>
> Not sure I understood yr problem though, so more details will help.
> -- ramki
>
> On 01/19/11 07:14, Keith McGuigan wrote:
>> Hello,
>>
>> In some recent changes to JVMTI breakpoint code, I added some code to 
>> update a cache during an oop_do() iteration over of methodOop.  The 
>> cache update involves dereferencing the methodOop's constMethodOop 
>> and getting the address to the code in it.
>>
>> Essentially, I'm doing something like this:
>>
>> JvmtiBreakpoint::oops_do(OopClosure* f) {
>>   for (int i = 0; i < _num_methods; ++i) {
>>     methodOop* m = method_at(i);
>>     f->do_oop(m);
>>
>>   // update the cache in case the methodOop moved
>>    _cache[i] =  (*m)->code_base() + _bci;
>>   }
>> }
>>
>> (The actual code is in GrowableCache::oops_do() in jvmtiImpl.cpp, but 
>> this is the general idea)
>>
>> Doing this assumes that the methodOops will always be valid and refer 
>> to the same method.  Considering what I'm seeing, I think this might 
>> be an invalid assumption.   When oops_do() is called from 
>> PSMarkSweep::mark_sweep_phase3(), I occasionally see that the 
>> resulting methodOop* refers to something else when I'm updating the 
>> cache (not sure what, but it doesn't appear to be the same 
>> methodOop).  It appears to be reset correctly in a later oops_do() 
>> iteration.
>>
>> Is this expected?  And if so, is there any way for me to tell when 
>> doing the oops_do() whether or not the resulting oops are valid?  
>> We're seeing assertion failures and crashes in nightly testing when 
>> the oop is not the methodOop that we expected.  I'll take a quick 
>> hack to make it work for now if we can fix it better later.
>>
>> If you've got anything for me to try, IM me (or email, but IM might 
>> be faster) and I'll give it a shot.
>>
>> -- 
>> - Keith