RFR(XS): 8222005: ClassRedefinition crashes with: guarantee(false) failed: OLD and/or OBSOLETE method(s) found

[coleen.phillimore at oracle.com]

Serguei,

Good find!!  The fix looks good.  I'm sure the optimization wasn't 
noticeable and thank you for the additional comments.

There is a Method::external_name() function that I believe prints all 
the things you want in the logging here:

http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/jvmti-redef.1/src/hotspot/share/oops/cpCache.cpp.udiff.html

I don't need to see another webrev if you make this change.

Thanks,
Coleen

On 5/14/20 12:26 PM, serguei.spitsyn at oracle.com wrote:
> Please, review a fix for The Kitchensink bug:
> https://bugs.openjdk.java.net/browse/JDK-8222005
>
> Webrev:
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/jvmti-redef.1/
>
> Summary:
>   The VM_RedefineClasses::doit()uses two helper classes to walk all VM 
> classes.
>   First is AdjustAndCleanMetadata to adjust method entries in the 
> vtables/itables/cpcaches.
>   Second is CheckClass to check that adjustments for all method 
> entries are correct.
>   The Kitchensink test is failing with two modes:
>     - guarantee(false) failed: OLD and/or OBSOLETE method(s) found in the
> VM_RedefineClasses::CheckClass::do_klass()
>     - SIGSEGV in the 
> ConstantPoolCacheEntry::get_interesting_method_entry() in context
>       of VM_RedefineClasses::CheckClass::do_klass() execution
>
>   The second failure mode is rare. In is before the first one in the 
> code path.
>   The root cause of both is that the 
> VM_RedefineClasses::AdjustAndCleanMetadata::do_klass()
>   is skipping the cpcache update for classes that are being redefined 
> assuming they are
>   being redefined by the current VM_RedefineClasses operation. In such 
> cases, the adjustment
>   is not needed as the cpcache is empty. The problem is that the 
> assumption above is wrong.
>   The class can also be redefined by another VM_RedefineClasses 
> operation which has already
>   executed its doit_prologue. The cpcache djustment for such class is 
> necessary.
>   The fix is to always call the cp_cache->adjust_method_entries() even 
> if the class is
>   being redefined by the current VM_RedefineClasses operation. It is 
> possible to skip it
>   but it will add extra complexity to the code.
>   The fix also includes minor tweak in the cpCache.cpp to include 
> method's class name to
>   the redefinition cpcache log.
>
> Testing:
>   Ran Kitchensink test locally on a Linux server with the 
> Instrumentation module enabled.
>   The test does not fail anymore.
>   In progress, a mach5 tiers 1-5 and runs and separate mach5 
> Kitchensink run.
>
> Thanks,
> Serguei

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20200515/92db214c/attachment.htm>