RFR(XS): JDK-6904403 assert(f == k->has_finalizer(), "inconsistent has_finalizer") with debug VM
coleen.phillimore at oracle.com
Tue May 27 15:47:28 UTC 2014
Ok, then. I don't need to see the copyright changes after you add
them. My code review is complete.
On 5/27/14, 11:45 AM, Dmitry Samersoff wrote:
> please see below.
> On 2014-05-27 19:02, Coleen Phillimore wrote:
>> This is excellent. Thank you for adding a test. I have a couple
>> comments but they are sort of simple things.
>> 1. In InstanceKlass::has_redefined_super() can you make "klass' an
>> InstanceKlass* and cast to InstanceKlass on line 1491 instead to avoid
>> the casting. You'll only have InstanceKlass on the way up.
> InstanceKlass klass->super() return Klass *, so we have to cast it anyway:
> klass = InstanceKlass::cast(klass->super());
> Current approach is a bit more readable on my view and similar to code
>> 2. The new test files need copyrights! Except maybe the manifest one
>> 3. Can you make jtreg commands to do the things in the script? I
>> didn't see anything very unusual.
> I tried it and ended up to bunch of java code instead of 10 lines of
> straightforward shell script.
> jtreg compiles classes with tools.jar from COMPILEJAVA, not from
> TESTJAVA. It might be a problem if COMPILE java doesn't exactly match
> TESTJAVA (e.g. cross compilation) and I didn't find a way to change it.
> I didn't find a way to instruct jtreg to build a jar file required for
> agent. I have to do it programatically and then use process builder to
> launch one more VM.
> So I would prefer to stay with a shell here.
>> Thank you for doing this. It's really helpful and it seems it found a
>> bug also (super classes?)
> Initially this fix was intended as very quick band-aid to address jcov
> problem important for SQE. But as soon as we go deeper I decide to make
> better fix to cover other possible cases with redefined finalizers.
>> On 5/24/14, 1:17 PM, Dmitry Samersoff wrote:
>>> This is new version of the fix.
>>> 1. Added testcase
>>> 2. Check entire class hierarchy for redefined classes
>>> as any of supers could be redefined and cause assert.
>>> On 2014-04-16 16:56, Dmitry Samersoff wrote:
>>>> Hi Everybody,
>>>> Please review small changes.
>>>> 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.
More information about the hotspot-runtime-dev