RFR 8082782: vm crash on StressRedefineWithoutBytecodeCorruption fails with assert(((Metadata*)obj)->is_, valid()) failed: obj is valid
Coleen Phillimore
coleen.phillimore at oracle.com
Thu Jul 23 01:25:01 UTC 2015
Dan,
Thank you for the quick code review.
On 7/22/15 8:35 PM, Daniel D. Daugherty wrote:
> On 7/22/15 10:20 AM, Coleen Phillimore wrote:
>> Summary: Walk compile task for Method* to not deallocate, store
>> methods in methodHandle while compile task is being created
>
> Wait... 'while compile task is being created'...
> You're holding these handles while the compile task
> is being removed... Am I confused here?
>
No, you're not confused, you are right, we're getting the compile task
is being taken off the compile queue.
>
>> I couldn't reproduce this or write a test with the exact timing
>> needed to get this crash, but this is what the stack trace and code
>> inspection tells me what happened. Reran all redefinition and
>> internal tonga 'quick' tests on the change with no regression.
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8082782.01/
>
> src/share/vm/compiler/compileBroker.hpp
> No comments.
>
> src/share/vm/compiler/compileBroker.cpp
> Is there a reason to have save_method and save_hot_method
> defined all the way up at lines 671-672 instead of
> being defined and init'ed on lines 710-711?
The methodHandle destructor has to run after the mutexLocker
destructor. The methodHandle destructors take it off the list of
Methods to mark on stack to not deallocate, but have to run after the
MutexLocker destructor so it has to be declared first.
>
> L708: // Save method pointer across
> Typo: 'pointer' -> 'pointers'
Fixed.
>
> So the idea of this fix is that you're holding methodHandles for
> these method pointers while removing the task because removing
> the task can go to a safepoint. While we're at that safepoint,
> a RedefineClasses call can come along and try and access/use the
> method pointers.
RedefineClasses will walk the previous versions of the method's holder
class and determine that nothing is using the old methods, and put them
on the list to deallocate the next time we unload classes. The
methodHandle marks these methods as being used (on_stack) across these
safepoints so they stick around.
> If we didn't have methodHandles on them, they
> could be freed by the time RedefineClasses tries to access/use
> them...
>
They'd be freed by the time the following code uses them (if you
redefine the class and subsequently do some class unloading GC, not
terribly likely but can happen in this small window and seems to have
happened in this crash from the dump). It's like a naked methodOop
before permgen elimination, except it doesn't move, it gets deallocated.
The is_valid() debugging function partially detects that something has
written over this bit of the metadata. Deallocation writes 0xf5 over
the stuff it deallocates.
> Do I have this right? Ouch and nice find.
>
thanks!
> src/share/vm/runtime/thread.cpp
> No comments.
>
>
Thank you!!
Coleen
>> bug link https://bugs.openjdk.java.net/browse/JDK-8082782
>>
>> Thanks,
>> Coleen
>
More information about the hotspot-dev
mailing list