Hi Coleen,
I have reviewed only the test portion of your change-set.
It was a good learning exercise for me on class transformation.
I have several comments:
1. Agent.java: redefine()
With your change, the block of code
Agent transformer = new Agent();
instrumentation.addTransformer(transformer, true);
will be called 3 times, thus adding the transformer 3 times.
Was it intended? If not, you could move this code to premain()
2. WalkThroughInvoke.java: stackWalk()
If the AccessControlException() is always expected, and making sure it
occurs is one of the test checks, I recommend throwing an exception
right after sm.checkMemberAccess(b, Member.DECLARED);
Such as:
try {
28 Class b = Object.class;
29 SecurityManager sm = new SecurityManager();
30 // walks the stack before it gets this exception.
31 // testing the stack walk with Method.invoke in the stack
32 sm.checkMemberAccess(b, Member.DECLARED);
* Misha>> throw new Exception("Test failed, the expected AccessControlException
did not occur");*
33 } catch (java.security.AccessControlException e) {
34 System.out.println("This exception is expected");
35 }
3. Style comment:
The style for Java code is to use 4 spaces for tabulation/indent
(please see the attached discussion with David Holmes).
Misha
On 8/8/2013 7:21 PM, Coleen Phillimore wrote:
Thanks, Serguei!
Coleen
On 08/08/2013 07:03 PM, serguei.spit...@oracle.com wrote:
Coleen,
The fix looks good, I do not see any issues.
Thanks,
Serguei
On 8/6/13 2:33 PM, Coleen Phillimore wrote:
Summary: ActiveMethodOopsCache was used to keep track of old
versions of some methods that are cached in Universe but is buggy
with permgen removal and not needed anymore
There was a crash in this function that I couldn't reproduce. It was
likely that the crash was for something else, but this is a lurking
bug.
open webrev at http://cr.openjdk.java.net/~coleenp/8009728/
bug link at http://bugs.sun.com/view_bug.do?bug_id=8009728
local bug link https://jbs.oracle.com/bugs/browse/JDK-8009728
Tested with vm.quick.testlist which includes redefine classes tests
and jck lang and vm tests.
Thanks,
Coleen
--- Begin Message ---
On 3/06/2013 9:52 PM, Mikhailo Seledtsov wrote:
Thank you David,
I have updated my editor to use 4 spaces as indentation, for Java. At
first I was not sure since some tests use 2 spaces, and some use 4.
There's a Java style guide somewhere, but you will find lots of code
that violates it. Going forward we use indent of 4 for Java code. While
hotspot uses an indent of 2.
Cheers,
David
As for opening '{' on the same line - it's a common Java style, I simply
missed it by mistake.
Misha
On 06/01/2013 08:00 AM, David Holmes wrote:
For future reference, there are a couple of style issues with the test:
a) open braces go on the same line as the keyword ie try {
b) indentation was wrong - use indent of 4 in Java sources
Sorry I didn't notice them before the push.
David
On 1/06/2013 4:13 AM, Mikhailo Seledtsov wrote:
On 05/31/2013 03:58 AM, David Holmes wrote:
Hi,
On 31/05/2013 3:25 AM, Mikhailo Seledtsov wrote:
Hi,
Please review this one-line fix,
Functional fix is fine. Please update the copyright year as this is
the policy for runtime.
and a test code to reproduce this bug.
Your test does not need:
* @library /testlibrary
or
import com.oracle.java.testlibrary.*;
No need to generate a new webrev though.
Thanks,
David (Reviewer)
http://cr.openjdk.java.net/~ctornqvi/webrev/6726963/webrev.00/
Overview:
the test case attempts to test out-of-memory condition
allocating
very large multi-dimensional array.
JVM can throw OOM exception, but should not crash or except in any
other
way.
Fix:
added a null-check when calling multi_allocate while allocating the
object in corresponding method
of the reflection class
Testing:
- Created a JTReg test case to reproduce this bug
- The test case fails before the fix, and passes after the fix.
Tested on 32-bit Linux build.
- Ran this test case on all supported platforms using JPRT
harness
- PASS
- Full JPRT run - PASS
- UTE run with vm.quick.testlist - NO new failures introduced by
this change
Thank you,
Misha
Hi David,
Thank you for the review.
I have incorporated the review changes, ran the updated test through
JPRT and will send the changes to Christian, who is sponsoring my change
for submission.
Thank you,
Misha
--- End Message ---
