RFR(XS): JDK-6904403 assert(f == k->has_finalizer(), "inconsistent has_finalizer") with debug VM

[serguei.spitsyn at oracle.com]

Hi Dmitry,

I agree with the approach in general, but think this fix is not fully 
correct:

  #ifdef ASSERT
    bool f = false;
    Method* m = k->lookup_method(vmSymbols::finalize_method_name(),
                                   vmSymbols::void_method_signature());
    if (m != NULL && !m->is_empty_method()) {
+    // Spec doesn't prevent agent from redefinition of empty finalyzer.
+    // despite the fact that it's generally bad idea and redefined finalyzer
+    // will not work as expected we shouldn't abort vm in this case
+    if (!k->has_been_redefined()) {
      f = true;
    }
+  }
    assert(f == k->has_finalizer(), "inconsistent has_finalizer");
  #endif


The issue is that if the class originally had a non-empty finalizer then 
after a redefinition
the 'f' will get value 'false' which should cause this assert to 
unreasonably fire again.

The fix you may want is to skip the assert if a class redefinition took 
place:

#ifdef ASSERT
   bool f = false;
   Method* m = k->lookup_method(vmSymbols::finalize_method_name(),
vmSymbols::void_method_signature());
   if (m != NULL && !m->is_empty_method()) {
       f = true;
   }
   // Spec doesn't prevent agent from redefinition of empty finalizer.
   // Despite the fact that it's generally bad idea and redefined finalizer
   // will not work as expected we shouldn't abort vm in this case
   if (!k->has_been_redefined()) {
     assert(f == k->has_finalizer(), "inconsistent has_finalizer");
   }
#endif

Also, please, note a couple of minor scratches in new comment:
  - finalyzer => finalizer
  - despite   => Despite

Thanks,
Serguei


On 4/16/14 5:56 AM, Dmitry Samersoff wrote:
> Hi Everybody,
>
> Please review small changes.
>
> http://cr.openjdk.java.net/~dsamersoff/JDK-6904403/webrev.01/
>
> This fix tells to JVM to don't assert if the class was redefined.
>
> Please notice, these changes doesn't make redefined finalyzers working.
> It's a large project which is out of scope of this changes.
>
> -Dmitry
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20140416/d6710f7b/attachment.html>