JVM/TI code review request (XS and M) (7182152)
Daniel D. Daugherty
daniel.daugherty at oracle.com
Wed Feb 6 12:08:03 PST 2013
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
>>>
>>>
>>>
>>
>>
>>
>
>
>
More information about the hotspot-runtime-dev
mailing list