Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-07-05 Thread Eric Wang
Hi Marks, Thanks for your great help to review and push my first fix.:-) Regards, Eric On 2012/7/6 6:19, Stuart Marks wrote: Pushed: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/97eb7a4b1fdd s'marks On 7/3/12 6:54 PM, Eric Wang wrote: Opps, It is my carelessness, I will be more careful in the

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-07-05 Thread Stuart Marks
Pushed: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/97eb7a4b1fdd s'marks On 7/3/12 6:54 PM, Eric Wang wrote: Opps, It is my carelessness, I will be more careful in the next bug fixes. Thank you to review and change it. Regards, Eric On 2012/7/4 8:58, Stuart Marks wrote: Sure, I can take care

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-07-03 Thread Eric Wang
Opps, It is my carelessness, I will be more careful in the next bug fixes. Thank you to review and change it. Regards, Eric On 2012/7/4 8:58, Stuart Marks wrote: Sure, I can take care of that. If this is the only change, no need to generate another webrev, Eric. s'marks On 7/3/12 4:53 PM, D

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-07-03 Thread Stuart Marks
Sure, I can take care of that. If this is the only change, no need to generate another webrev, Eric. s'marks On 7/3/12 4:53 PM, David Holmes wrote: Please fix the missing space in if(ref.get() before pushing. Thanks, David On 4/07/2012 7:04 AM, Stuart Marks wrote: Webrev now posted at h

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-07-03 Thread David Holmes
Please fix the missing space in if(ref.get() before pushing. Thanks, David On 4/07/2012 7:04 AM, Stuart Marks wrote: Webrev now posted at http://cr.openjdk.java.net/~smarks/yiming.wang/6948101/webrev.2/ The changes look good to me. If there's no further discussion, I'll push them in a coupl

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-07-03 Thread Stuart Marks
Webrev now posted at http://cr.openjdk.java.net/~smarks/yiming.wang/6948101/webrev.2/ The changes look good to me. If there's no further discussion, I'll push them in a couple days (U.S. holiday coming up). s'marks On 7/3/12 12:20 AM, Stuart Marks wrote: This approach looks good to me. Plea

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-07-03 Thread Stuart Marks
This approach looks good to me. Please go ahead and update 7123972 as well, and I'll post these webrevs tomorrow (my time). Thanks. s'marks On 7/2/12 11:41 PM, Eric Wang wrote: David, I have added the comments before the loop, please help to review. If it is OK, I'll update the 7123972 as we

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-07-02 Thread Eric Wang
David, I have added the comments before the loop, please help to review. If it is OK, I'll update the 7123972 as well. Thanks, Eric On 2012/7/3 12:22, David Holmes wrote: Hi Eric, On 3/07/2012 1:28 PM, Eric Wang wrote: Hi Stuart and David, Thanks for the suggestion about the timeout. so t

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-07-02 Thread David Holmes
Hi Eric, On 3/07/2012 1:28 PM, Eric Wang wrote: Hi Stuart and David, Thanks for the suggestion about the timeout. so there are three ways to handle the timeout. 1. Add comment to explain that test failed due to default timeout 2. Specify the the timeout option 3. Timeout programly Is it ok fo

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-07-02 Thread Eric Wang
Hi Stuart and David, Thanks for the suggestion about the timeout. so there are three ways to handle the timeout. 1. Add comment to explain that test failed due to default timeout 2. Specify the the timeout option 3. Timeout programly Is it ok for you that i'd like to choose the second way "ex

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-07-02 Thread David Holmes
On 3/07/2012 7:08 AM, Stuart Marks wrote: On 7/1/12 5:39 PM, David Holmes wrote: Now, how can the test fail? If ref is never cleared, the while loop will never terminate, and we rely on jtreg to timeout and kill this test. It would be good to have a brief comment above the while loop that explai

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-07-02 Thread Stuart Marks
On 7/1/12 5:39 PM, David Holmes wrote: Now, how can the test fail? If ref is never cleared, the while loop will never terminate, and we rely on jtreg to timeout and kill this test. It would be good to have a brief comment above the while loop that explains this. Agreed. Something like // Might

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-07-01 Thread David Holmes
On 30/06/2012 8:00 AM, Stuart Marks wrote: Thanks for updating this. I've posted the revised webrev: http://cr.openjdk.java.net/~smarks/yiming.wang/6948101/webrev.1/ It's good that this is getting simpler in that we don't have to deal with finalization. But it can still get simpler The code

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-06-29 Thread Stuart Marks
Thanks for updating this. I've posted the revised webrev: http://cr.openjdk.java.net/~smarks/yiming.wang/6948101/webrev.1/ It's good that this is getting simpler in that we don't have to deal with finalization. But it can still get simpler The code is in the webrev but I'll also paste it h

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-06-29 Thread Eric Wang
Hi David, I have made changes by following your suggestion. Can you please help to review again? Thanks a lot! I also change the code of 7123972 which is sent in another mail. Regards, Eric On 2012/6/29 11:04, David Holmes wrote: On 29/06/2012 12:10 PM, Eric Wang wrote: Hi Stuart, Thank yo

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-06-28 Thread David Holmes
On 29/06/2012 12:10 PM, Eric Wang wrote: Hi Stuart, Thank you, The fix looks simpler than the previous one, it shoud be better to add a Thread.sleep(20) in the loop to avoid multiple calls on System.gc(); Oops I missed the sleep out of my suggestion too. Here is a small question if it is pos

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-06-28 Thread Eric Wang
Hi Stuart, Thank you, The fix looks simpler than the previous one, it shoud be better to add a Thread.sleep(20) in the loop to avoid multiple calls on System.gc(); Here is a small question if it is possible that after GC, the strong ref remains but somehow weakref is broken??so the loop will t

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-06-28 Thread David Holmes
Hi Stuart, I don't think finalization or reference queues are needed in this case. We could simply do: while(true) { System.gc(); if (weakRef.get() == null) break; } We have to remember that this is not testing the correct operation of weak references, nor of finalizatio

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-06-28 Thread Stuart Marks
And here's the webrev for this one: http://cr.openjdk.java.net/~smarks/yiming.wang/6948101/webrev.0/ Also looks good to me. It's pretty similar to the other fix (7123972). If there are no further comments I'll push this in the next day or so. * * * Some further comments, mainly aimed at the

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-06-27 Thread Eric Wang
Hi David, Thank you for review! Hi Stuart, Can you please help to review and post the webrev, Thanks a lot! Eric On 2012/6/27 15:32, David Holmes wrote: On 27/06/2012 4:57 PM, Eric Wang wrote: Hi David & Stuart, Thank you for the help! please review the in webrev 6948101.zip in attachment.

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-06-27 Thread David Holmes
On 27/06/2012 4:57 PM, Eric Wang wrote: Hi David & Stuart, Thank you for the help! please review the in webrev 6948101.zip in attachment. Thanks Eric. That fix also seems fine to me. David Regards, Eric On 2012/6/27 9:14, Stuart Marks wrote: Hi Eric, Alan Bateman asked me to help you ou

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-06-26 Thread Eric Wang
Hi David & Stuart, Thank you for the help! please review the in webrev 6948101.zip in attachment. Regards, Eric On 2012/6/27 9:14, Stuart Marks wrote: Hi Eric, Alan Bateman asked me to help you out with this since he's going to be unavailable for a couple weeks. I didn't see you on the O

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-06-26 Thread Stuart Marks
Hi Eric, Alan Bateman asked me to help you out with this since he's going to be unavailable for a couple weeks. I didn't see you on the OpenJDK census [1] so you might not have an Author role and thus the ability to post webrevs. If indeed you don't, I can post a webrev for you. I can also p

Re: [PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-06-26 Thread David Holmes
Hi Eric, On 26/06/2012 7:26 PM, Eric Wang wrote: Please help to review the fix attached for test bug 6948101 which is same root cause as bug 7123972 . The test makes wrong assumption that GC is

[PATCH] Review Request - Test bug: 6948101 java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-06-26 Thread Eric Wang
Hi All, Please help to review the fix attached for test bug 6948101 which is same root cause as bug 7123972 . The test makes wrong assumption that GC is started immediately to recycle unused