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

Daniel Fuchs daniel.fuchs at oracle.com
Wed Dec 17 09:55:16 UTC 2014


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
>>
>



More information about the serviceability-dev mailing list