I agree, the test is a piece of art - nice example to start from for
future test development.
Ship it!
Thanks,
Serguei
On 2/6/13 12:08 PM, Daniel D. Daugherty wrote:
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: <begin output.log>"
102a106,107
> echo "INFO: <end output.log>"
>
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
> esac
>
> PASS2_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
> esac
>
> if [ "$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/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
