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

[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 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 "explict
>> timeout" as the test looks simply and lucid?
>
> If by #2 you mean add something like:
>
> @run main/timeout=10
>
> then no. Earlier this year there were changes to a bunch of tests that 
> set timeouts shorter than the jtreg default, to delete the timeouts. 
> Given that, my misgivings about relying on the harness are not 
> relevant so we need only do #1 and add a comment to make it clear that 
> a failing test will be indicated via the harness timeout.
>
> David
> -----
>
>> Regards,
>> Eric
>>
>> On 2012/7/3 10:38, David Holmes wrote:
>>> 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
>>>>>> explains
>>>>>> this.
>>>>>
>>>>> Agreed. Something like
>>>>>
>>>>> // Might require multiple calls to System.gc() for weak-references
>>>>> processing
>>>>> to be complete.
>>>>> // If the weak-reference is not cleared as expected we will hang here
>>>>> // until timed out by the test harness
>>>>>
>>>>> Though now I write that I'm unhappy that this will only behave nicely
>>>>> if run
>>>>> from jtreg. We do use explicit timeouts in other tests.
>>>>
>>>> So, are you unhappy enough that we should ask Eric to add an explicit
>>>> timeout?
>>>
>>> No. But I'm not the authoritive voice in this area :)
>>>
>>>> I don't think it would be that difficult. Perhaps a technique like the
>>>> following would suffice. Get System.currentTimeMillis() before the 
>>>> loop,
>>>> and call it again each time through the loop, and fail if the 
>>>> difference
>>>> is greater than the timeout (e.g., 60 sec). Doesn't involve other
>>>> threads or even Timer/TimerTask.
>>>
>>> Or simply limit the number of loops so that N*sleepTime = timeout.
>>> This then requires adding back the check for null outside the loop.
>>>
>>>> On the other hand if one is running this test directly instead of
>>>> through jtreg, one can just ^C it if it's taking an unexpectedly long
>>>> time.
>>>
>>> True.
>>>
>>> David
>>> -----
>>>
>>>
>>>> s'marks
>>
>>


-------------- next part --------------
--- old/test/ProblemList.txt	2012-07-03 14:21:56.063520415 +0800
+++ new/test/ProblemList.txt	2012-07-03 14:21:54.846424695 +0800
@@ -271,9 +271,6 @@
 # 7140992
 java/rmi/server/Unreferenced/finiteGCLatency/FiniteGCLatency.java generic-all
 
-# 6948101
-java/rmi/transport/pinLastArguments/PinLastArguments.java	generic-all
-
 # 7146541
 java/rmi/transport/rapidExportUnexport/RapidExportUnexport.java	linux-all
 
-------------- next part --------------
--- old/test/java/rmi/transport/pinLastArguments/PinLastArguments.java	2012-07-03 14:22:04.443420594 +0800
+++ new/test/java/rmi/transport/pinLastArguments/PinLastArguments.java	2012-07-03 14:22:02.403420591 +0800
@@ -78,10 +78,15 @@
         }
         impl = null;
 
-        System.gc();
-
-        if (ref.get() != null) {
-            throw new Error("TEST FAILED: impl not garbage collected");
+        // Might require multiple calls to System.gc() for weak-references
+        // processing to be complete. If the weak-reference is not cleared as
+        // expected we will hang here until timed out by the test harness. 
+        while (true) {
+            System.gc();
+            Thread.sleep(20);
+            if(ref.get() == null) {
+                break;
+            }
         }
 
         System.err.println("TEST PASSED");