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

[serguei.spitsyn at oracle.com]

Looks good.

Thanks,
Serguei

On 4/17/14 5:56 AM, Dmitry Samersoff wrote:
> Serguei,
>
> Updated webrev:
>
> http://cr.openjdk.java.net/~dsamersoff/JDK-6904403/webrev.02/
>
>
> On 2014-04-17 02:56, serguei.spitsyn at oracle.com wrote:
>> 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
>>>
>