Hi Misha,
Thank you for reviewing the test.
On 8/9/2013 12:49 PM, Mikhailo Seledtsov wrote:
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()
It wasn't intended. I meant to move that to premain and have moved it
and the test still passes.
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 }
I don't think I want to do this change because if we change whether the
checkMemberAccess() throws an exception, which seems unlikely, the test
will start failing. And so someone will have to fix the test for
something that it's not trying to test.
3. Style comment:
The style for Java code is to use 4 spaces for tabulation/indent
(please see the attached discussion with David Holmes).
I changed the files to have 4 space indentation.
Thanks!
Coleen
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
