Dan, All 3 versions of the code looks good. Thank you for enabling the printing for product since this type of problem is so hard to duplicate.
A small note, I think it would have been easier for the internal code logic for the CPCE::check_no_old_or_obsolete_entries to reverse the true/false, but no need to change. I would appreciate the comment from is_interesting_method_entry copied to check_no_old_or_obsolete_entries about virtual and final that f2 contains a method ptr instead of a vtable index. In the jdk8 version in cpCache.cpp you've added the is_valid checks for metadata. For a future cleanup, do we need f2_as_vfinal_method and is_interesting_method_entry to do that as well? Is redefineclasses supported in the MinimalVM? thanks, Karen On Feb 1, 2013, at 2: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 >
