JVM/TI code review request (XS and M) (7182152)

[Daniel D. Daugherty]

On 2/5/13 10:50 AM, serguei.spitsyn at oracle.com wrote:
> Both tests look good, I do not see any issues yet.
> What is missed is a comment explaining what happened
> when the bug is not fixed and what correct behavior is expected.
> Maybe it'd make sense to put bad and good output into the comment,
> not everything, but the most important fragments.
> It would help a lot to understand what test is doing.
Serguei,

Thanks for the review!

I put sample failures for both tests into the bug (8007420) a couple
of days ago. I'd prefer not to clutter up the test with sample outputs
if that's OK with you.

Dan


>
>
> Thanks,
> Serguei
>
>
> On 2/1/13 3:55 PM, Daniel D. Daugherty wrote:
>> And here is the webrev for the new tests (relative to JDK8-T&L):
>>
>> http://cr.openjdk.java.net/~dcubed/8007420-webrev/0-jdk8-tl/
>>
>> As always, comments and suggestions are welcome.
>>
>> Dan
>>
>>
>> On 2/1/13 4:39 PM, Daniel D. Daugherty wrote:
>>> > There are two new tests that will be pushed to the JDK repos using
>>> > a different bug ID (not yet filed):
>>>
>>> New bug is now filed:
>>>
>>>     8007420 add test for 6805864 to com/sun/jdi, add test for 7182152
>>>             to java/lang/instrument
>>>     http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8007420
>>>     https://jbs.oracle.com/bugs/browse/JDK-8007420
>>>
>>> Of course, the tests cannot be pushed until the HSX changes have made
>>> it into a promoted build and thus available to JDK8-T&L.
>>>
>>> Dan
>>>
>>>
>>> On 2/1/13 12:55 PM, Daniel D. Daugherty wrote:
>>>> Greetings,
>>>>
>>>> I have a fix for the following JVM/TI bug:
>>>>
>>>>     7182152 Instrumentation hot swap test incorrect monitor count
>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7182152
>>>>     https://jbs.oracle.com/bugs/browse/JDK-7182152
>>>>
>>>> The fix for the bug in the product code is one line:
>>>>
>>>> src/share/vm/oops/klassVtable.cpp:
>>>>
>>>> @@ -992,18 +1020,50 @@
>>>>            // RC_TRACE macro has an embedded ResourceMark
>>>>            RC_TRACE(0x00200000, ("itable method update: %s(%s)",
>>>>              new_method->name()->as_C_string(),
>>>>              new_method->signature()->as_C_string()));
>>>>          }
>>>> -        break;
>>>> +        // cannot 'break' here; see for-loop comment above.
>>>>        }
>>>>        ime++;
>>>>      }
>>>>    }
>>>>  }
>>>>
>>>> and is applicable to JDK7u10/HSX-23.6 and JDK7u14/HSX-24. Coleen
>>>> already fixed the bug as part of the Perm Gen Removal (PGR) project
>>>> in HSX-25. Yes, we found a 1-line bug fix buried in the monster PGR
>>>> changeset. Many thanks to Coleen for her help in this bug hunt!
>>>>
>>>> The rest of the code in the webrevs are:
>>>>
>>>> - additional JVM/TI tracing code backported from Coleen's PGR 
>>>> changeset
>>>> - additional JVM/TI tracing code added by me and forward ported to 
>>>> HSX-25
>>>> - a new -XX:TraceRedefineClasses=16384 flag value for finding these
>>>>   elusive old or obsolete methods
>>>> - exposure of some printing code to the PRODUCT build so that the new
>>>>   tracing is available in a PRODUCT build
>>>>
>>>> You might be wondering why the new tracing code is exposed in a 
>>>> PRODUCT
>>>> build. Well, it appears that more and more PRODUCT bits deployments 
>>>> are
>>>> using JVM/TI RedefineClasses() and/or RetransformClasses() at run-time
>>>> to instrument their systems. This bug (7182152) was only 
>>>> intermittently
>>>> reproducible in the WLS environment in which it occurred so I made the
>>>> tracing available in a PRODUCT build to assist in the hunt.
>>>>
>>>> Raj from the WLS team has also verified that the HSX-23.6 version of
>>>> fix resolves the issue in his environment. Thanks Raj!
>>>>
>>>> Here are the URLs for the three webrevs:
>>>>
>>>> http://cr.openjdk.java.net/~dcubed/7182152-webrev/0-hsx23.6/
>>>> http://cr.openjdk.java.net/~dcubed/7182152-webrev/0-hsx24/
>>>> http://cr.openjdk.java.net/~dcubed/7182152-webrev/0-hsx25/
>>>>
>>>> I have run the following test suites from the JPDA stack on the
>>>> JDK7u10/HSX-23.6 version of the fix with 
>>>> -XX:TraceRedefineClasses=16384
>>>> specified:
>>>>
>>>>     sdk-jdi
>>>>     sdk-jdi_closed
>>>>     sdk-jli
>>>>     vm-heapdump
>>>>     vm-hprof
>>>>     vm-jdb
>>>>     vm-jdi
>>>>     vm-jdwp
>>>>     vm-jvmti
>>>>     vm-sajdi
>>>>
>>>> The tested configs are:
>>>>
>>>>     {Solaris-X86, WinXP}
>>>>       X {Client VM, Server VM}
>>>>       X {-Xmixed, -Xcomp}
>>>>       X {product, fastdebug}
>>>>
>>>> With the 1-liner fix in place, the new tracing code does not find any
>>>> instances of this failure mode in any of the above test suites. 
>>>> Without
>>>> the the 1-liner fix in place, the new tracing code finds one instance
>>>> of this failure mode in the above test suites:
>>>>
>>>>     test/java/lang/instrument/IsModifiableClassAgent.java
>>>>
>>>> There are two new tests that will be pushed to the JDK repos using
>>>> a different bug ID (not yet filed):
>>>>
>>>>     test/com/sun/jdi/RedefineAbstractClass.sh
>>>> test/java/lang/instrument/RedefineSubclassWithTwoInterfaces.sh
>>>>
>>>> There will be a separate review request for the new tests.
>>>>
>>>> I'm currently running the JPDA stack of tests on the JDK7u14/HSX-24
>>>> and JDK8-B75/HSX-25 versions of the fix. That testing will likely
>>>> take all weekend to complete.
>>>>
>>>> Thanks, in advance, for any comments and/or suggestions.
>>>>
>>>> Dan
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>