JVM/TI code review request (XS and M) (7182152) (original) (raw)
Daniel D. Daugherty daniel.daugherty at oracle.com
Wed Feb 6 12:08:03 PST 2013
- Previous message: JVM/TI code review request (XS and M) (7182152)
- Next message: JVM/TI code review request (XS and M) (7182152)
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Greetings,
I've revised the tests based on Coleen's and Serguei's feedback in Code Review Round 0.
Here is the URL for the webrev for Code Review Round 1:
http://cr.openjdk.java.net/~dcubed/8007420-webrev/1-jdk8-tl/
Summary of the changes:
- RedefineAbstractClass.sh - cleaned up the header comment
- RedefineAbstractClass.sh
- more clear description of what's being tested
- add comments describing what the guarantee failure looks like
- add checks for proper pre-RedefineClasses output
- add checks for proper post-RedefineClasses output
- RedefineSubclassWithTwoInterfacesApp.java
- RedefineSubclassWithTwoInterfacesRemote.java
- refactor the test app's echo() method call into echo1() and echo2() so it is easier to check for the failure mode
- RedefineSubclassWithTwoInterfacesImpl.java
- RedefineSubclassWithTwoInterfacesImpl_1.java
- add comment to explain why these sources are identical
Diffs are below since the webrev above is relative to JDK8-T&L and everything is "new".
Dan
Here are diffs (ignoring whitespace) to make it clear what has changed between Code Review Round 0 and Code Review Round 1:
$ diff -w test/com/sun/jdi/RedefineAbstractClass.sh{.cr0,} 31a32
@author Daniel D. Daugherty
$ diff -w test/java/lang/instrument/RedefineSubclassWithTwoInterfaces.sh{.cr0,} 26c26,27 < # @summary Redefining a subclass that implements two interfaces is broken.
@summary Redefine a subclass that implements two interfaces and
verify that the right methods are called.
101a103,104
echo "INFO: " 102a106,107 echo "INFO: "
108,112c113,127 < MESG="guarantee" < grep "$MESG" output.log < result=$? < if [ "$result" = 0 ]; then < echo "FAIL: found '$MESG' in the test output"
When this bug manifests, RedefineClasses() will fail to update
one of the itable entries to refer to the new method. The log
will include the following line when the bug occurs:
guarantee(false) failed: OLD and/or OBSOLETE method(s) found
If this guarantee happens, the test should fail in the status
check above, but just in case it doesn't, we check for "guarantee".
FAIL_MESG="guarantee" grep "$FAIL_MESG" output.log status=$? if [ "$status" = 0 ]; then echo "FAIL: found '$FAIL_MESG' in the test output." 115c130,131 < echo "PASS: did NOT find '$MESG' in the test output"
echo "INFO: did NOT find '$FAIL_MESG' in the test output." # be optimistic here
118a135,164 PASS1_MESG="before any redefines" cnt=
grep "$PASS1_MESG" output.log | grep 'version-0' | wc -l
case "$cnt" in 2) echo "INFO: found 2 version-0 '$PASS1_MESG' mesgs." ;; *) echo "FAIL: did NOT find 2 version-0 '$PASS1_MESG' mesgs." echo "INFO: grep '$PASS1_MESG' output:" grep "$PASS1_MESG" output.log result=1 esacPASS2_MESG="after redefine" cnt=
grep "$PASS2_MESG" output.log | grep 'version-1' | wc -l
case "$cnt" in 2) echo "INFO: found 2 version-1 '$PASS2_MESG' mesgs." ;; *) echo "FAIL: did NOT find 2 version-1 '$PASS2_MESG' mesgs." echo "INFO: grep '$PASS2_MESG' output:" grep "$PASS2_MESG" output.log result=1 esacif [ "$result" = 0 ]; then echo "PASS: test passed both positive and negative output checks." fi
$ diff test/java/lang/instrument/RedefineSubclassWithTwoInterfacesApp.java{.cr0,} 43,45c43,45 < // make an echo() call before any redefinitions: < mesg = remote.echo("test message #0"); < System.out.println("RedefineSubclassWithTwoInterfacesApp: mesg='"
// make echo() calls before any redefinitions: mesg = remote.echo1("test message #1.1");
System.out.println("RedefineSubclassWithTwoInterfacesApp: echo1 mesg='" 46a47,49 mesg = remote.echo2("test message #2.1"); System.out.println("RedefineSubclassWithTwoInterfacesApp: echo2 mesg='" + mesg + "' before any redefines"); 54,56c57,62 < mesg = remote.echo("test message #1"); < System.out.println("RedefineSubclassWithTwoInterfacesApp: mesg='" < + mesg + "' after redefine #1");
mesg = remote.echo1("test message #1.2");
System.out.println("RedefineSubclassWithTwoInterfacesApp: echo1 mesg='" + mesg + "' after redefine"); mesg = remote.echo2("test message #2.2"); System.out.println("RedefineSubclassWithTwoInterfacesApp: echo2 mesg='" + mesg + "' after redefine");
$ diff test/java/lang/instrument/RedefineSubclassWithTwoInterfacesImpl.java{.cr0,} 23a24,27
// Reproducing this bug only requires an EMCP version of the // RedefineSubclassWithTwoInterfacesImpl class so // RedefineSubclassWithTwoInterfacesImpl.java and // RedefineSubclassWithTwoInterfacesImpl_1.java are identical.
$ diff test/java/lang/instrument/RedefineSubclassWithTwoInterfacesImpl_1.java{.cr0,} 23a24,27
// Reproducing this bug only requires an EMCP version of the // RedefineSubclassWithTwoInterfacesImpl class so // RedefineSubclassWithTwoInterfacesImpl.java and // RedefineSubclassWithTwoInterfacesImpl_1.java are identical.
$ diff test/java/lang/instrument/RedefineSubclassWithTwoInterfacesRemote.java{.cr0,} 40,41c40,41 < public String echo(String s) { < return "intf1<" + intf1.echo(s) + "> intf2< " + intf2.echo(s) + ">";
public String echo1(String s) { return "intf1<" + intf1.echo(s) + ">";
42a43,46
public String echo2(String s) { return "intf2<" + intf2.echo(s) + ">"; }
On 2/1/13 4: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/viewbug.do?bugid=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/viewbug.do?bugid=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 @@ // RCTRACE macro has an embedded ResourceMark RCTRACE(0x00200000, ("itable method update: %s(%s)", newmethod->name()->asCstring(), newmethod->signature()->asCstring())); } - 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-jdiclosed 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
- Previous message: JVM/TI code review request (XS and M) (7182152)
- Next message: JVM/TI code review request (XS and M) (7182152)
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]