Re: [PATCH] bug fix for 6959636 testcase failing on windows javax/management/loading/LibraryLoader/LibraryLoaderTest.java

[David Holmes]

On 3/09/2012 4:54 PM, Eric Wang wrote:

Hi David,

Good catch, I have removed the try-catch block, please help to review, I
can confirm the fix is just to run in othervm.

That's the important part :)

http://dl.dropbox.com/u/90659131/fixes/6959636/webrev/index.html

Cleanup looks fine to me.

David

Thanks,
Eric
On 2012/9/3 13:47, David Holmes wrote:

Hi Eric,

On 3/09/2012 3:42 PM, Eric Wang wrote:

Hi David,

Thanks for your comments, I have updated the fix based on your
suggestion, Can you please help to review again.
http://dl.dropbox.com/u/90659131/fixes/6959636/webrev/index.html

You don't need the try/catch here:

137 try {
138 ObjectName mbean =
139 ((ObjectInstance) res[j]).getObjectName();
140 result = server.getAttribute(mbean, "Random");
141 System.out.println("MBean #" + j + " = " + mbean);
142 System.out.println("Random number = " + result);
143 } catch (ReflectionException e) {
144 throw e;
145 }

And can you please confirm that the actual fix here is just to run in
othervm?

Thanks,
David

Thanks,
Eric
On 2012/9/3 11:45, David Holmes wrote:

Hi Eric,

On 3/09/2012 1:22 PM, Eric Wang wrote:

Hi All,

I am an engineer from SQE team who is helping to cleanup ProblemList.
Here is a fix below for bug 6959636
<http://monaco.us.oracle.com/detail.jsf?cr=6959636>, Can you please
help
to review?
http://dl.dropbox.com/u/90659131/fixes/6959636/webrev/index.html

The test failed when it is executed by the command "jtreg -agentvm
-retain:fail,error", root cause is "-retain:fail,error" option
tries to
remove temp files created by test but they are still opened by the
agentvm/samevm.

So the fix for that is to run in othervm?

And the code changes are actually nothing to do with the original
problem? You are just simplifying the error logic by having the
exceptions propagate?

Here:

130 System.err.println("Failed to load the MBean #" + j +
131 ". The shown Throwable was caught.");

and here:

145 } catch (ReflectionException e) {
146 System.err.println("A ReflectionException, wrapping " +
147 "the shown exception, occured when" +

The printed messages no longer makes sense because there is no "shown
exception" anymore. I'd suggest just getting rid of the println's and
let the exceptions propagate directly - no need for any try/catch now
that main throws Throwable.

David

Thanks,
Eric