jmx-dev Codereview: JDK-8065764 javax/management/monitor/CounterMonitorTest.java hangs

[Jaroslav Bachorik]

On 12/02/2014 02:40 PM, shanliang wrote:
> Jaroslav Bachorik wrote:
>> On 12/01/2014 02:50 PM, shanliang wrote:
>>> Hi,
>>> please review this test bug fix:
>>>
>>> webrev:
>>> http://cr.openjdk.java.net/~sjiang/JDK-8065764/00/
>>>
>>> bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8065764
>>>
>>
>> test/javax/management/monitor/CounterMonitorTest.java
>> L61 - observedValue could be Integer
> Could be, but not make difference, observedValue is only used with the
> operation "==", like
>     while (value != observedValue)

In that case having a correct type would make even more sense for the 
readability.

>> L225-238 - you could replace this block with the usage of Phaser (if
>> you'd do that you could completely remove 'observedValue')
> Not sure that it is a good idea to remove "observedValue" by using
> Phaser, yes using Phaser can tell when the monitor does an observation,
> but it is better to know which count value the monitor observed, in case
> the thread was waked up accidentally.

I don't think phaser can be confused by spurious wake-ups.

If you want to keep the current way of synchronizing then you should 
make the 'count' variable volatile (L77) - it might be read and written 
to from different threads.

-JB-

>
> Thanks,
> Shanliang
>>
>> -JB-
>>
>>
>>> The test tested the mode "difference", according to the Javadoc:
>>>       If the counter difference mode is used, the value of the derived
>>> gauge is calculated as the difference between the observed counter
>>> values for two successive observations.
>>>
>>> The test set the first value and then waited 2 times of
>>> granularityperiod at line 171, hoped that the monitor would get the
>>> first observation during this waiting time, but the test could fail
>>> because granularityperiod * 2 was not enough and the test did the second
>>> set before the monitor did the first observation.
>>>
>>> It is easy to make the test timeout by commenting out the line 171.
>>>
>>> The proposed solution is to get informed when the monitor did
>>> observation on calling:
>>>     StdObservedObject.getNbObjects();
>>>
>>> Thanks,
>>> Shanliang
>>>
>>>
>>
>