RFR(S): 6977426: sun/tools tests can intermittently fail to find app's Java pid

[Daniel Fuchs]

On 17/12/14 12:05, Yekaterina Kantserova wrote:
> Daniel,
>
> It's really funny to get such a feedback!
>
> (1) The output from test right now looks like:
>
> ----------System.out:(7/183)----------
> call count = 1000
> instance count: 1001
> call count = 2000
> instance count: 1001
> Finishing early due to non-increasing instance count
> increasing count: 1
> decreasing or the same count: 1
>
> so printing the value of 'count' is already in there: 'call count ='.

I meant to print the final value for count - but OK - having the
'call count' printed every 1000 count should be enough :-)


> (2) In the case we want to have an extra condition I think it should be
> 'count > 2000'. Otherwise if the end time is passed the 'count > 1000'
> will result in having just one iteration (we will left the loop on 1100)
> and test will fail on assert.

right. my mistake. you need at least two histograms.

> The test timeout value should be increased as well. Right now it's 120 +
> 30 sec.
>
>   * @run main/timeout=150 TestLoggerWeakRefLeak Logger
>
> If the test will not be completed in 120 s the extra 30 s will not make
> the trick and JTreg will interrupt it. What do you think will be enough?

I am not sure - but I guess what you have now should be good enough
for starter. In the problematic configurations there often is
an additional timeout factor - so in such case the real jtreg timeout
would I believe be something like 150*4 anyway - which should hopefully
leave time enough for the while loop to take two histograms. Simply
adding the extra condition to keep looping until we have at least two
should do the trick.

best regards, and thanks again for taking care of 6977426!

-- daniel

>
> // Katja
>
>
>
> On 12/17/2014 10:55 AM, Daniel Fuchs wrote:
>> Hi Katja,
>>
>> Sorry for not thinking of that when I replied earlier.
>> Your new test is so much more readable than the old shell
>> script :-)
>>
>> These are minor  suggestions but they might help analysis
>> if the test fails:
>>
>> On 17/12/14 10:10, Yekaterina Kantserova wrote:
>>> Hi,
>>>
>>> Thanks for all reviews!
>>>
>>> The new webrev can be found here:
>>> http://cr.openjdk.java.net/~ykantser/6977426/webrev.01/
>>
>> Not that I believe it's very likely to happen, but I wonder
>> if the condition to exit the while loop:
>>
>>   79         while (now < (startTime + (LOOP_TIME_SEC * 1000))) {
>>
>> should also make sure that count > 1000 - and continue if not.
>> (Thinking of passed timeout related issues when tests where run
>>  with -Xcomp on fastdebug builds during integration nightlies)
>>
>> Possibly printing the value of 'count' before the assert
>> at line 103 could help with debugging too, if the test actually
>> fails (even though the combination of increasingCount
>> + decreasingCount would give us a hint)
>>
>> best regards,
>>
>> -- daniel
>>
>>>
>>> // Katja
>>>
>>>
>>>
>>> On 12/16/2014 05:41 PM, Jaroslav Bachorik wrote:
>>>> Hi Katja,
>>>>
>>>> This request should probably go to core-libs as well.
>>>>
>>>> Other than that I have just a few nits:
>>>>
>>>> test/java/util/logging/TestLoggerWeakRefLeak.java
>>>>
>>>> L57: if ("Logger".contains(args[0])) -> "Logger".equals(args[0]) ?
>>>> L127: // in LogManager.loggers that *arebeing* properly managed ->
>>>> *are being*
>>>>
>>>> callLoggerAndCount() and callAnonymousLoggerAndCount() are using the
>>>> 'count' variable to always return 100. Could they be made to return a
>>>> constant instead?
>>>>
>>>>
>>>> Cheers,
>>>>
>>>> -JB-
>>>>
>>>>
>>>> On 12/16/2014 04:57 PM, Yekaterina Kantserova wrote:
>>>>> Hi,
>>>>>
>>>>> Could I please have a review of this fix.
>>>>>
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-6977426
>>>>> webrev: http://cr.openjdk.java.net/~ykantser/6977426/webrev.00/
>>>>>
>>>>> java/util/logging/LoggerWeakRefLeak.sh and
>>>>> java/util/logging/AnonLoggerWeakRefLeak.sh are merged and rewritten in
>>>>> java. sun/tools/common/CommonTests.sh is removed because it doesn't
>>>>> test
>>>>> the product but sun/tool/common library functionality.
>>>>>
>>>>> Thanks,
>>>>> Katja
>>>>
>>>
>>
>