[9] RFR(S): 8080999: MemoryPoolMXBean.getUsageThresholdCount() returns incorrect value

[Tobias Hartmann]

[CC'ing runtime for wider audience]


On 03.09.2015 11:33, Tobias Hartmann wrote:
> Hi,
> 
> please review the following patch.
> 
> https://bugs.openjdk.java.net/browse/JDK-8080999
> http://cr.openjdk.java.net/~thartmann/8080999/webrev.00/
> 
> Problem:
> The compiler test [1] sets a maximum usage threshold for a code heap memory pool, allocates enough memory to hit the threshold and checks if the threshold counter was incremented (see 'CodeCacheUtils.hitUsageThreshold' [2]). On success, the allocated memory is freed and we start over again. The counter should only be incremented the first time the threshold is crossed. Subsequent threshold hits are only counted if the usage fell below the threshold value. The test times out waiting for the threshold counter to be incremented.
> 
> The problem is that there is a race between SensorInfo::set_gauge_sensor_level() which checks if the sensor should be triggered and SensorInfo::clear() which resets the sensor once the usage falls below the threshold value. The race occurs if SensorInfo::process_pending_requests() invokes SensorInfo::clear() because there is a pending request to clear the sensor. In the meantime SensorInfo::set_gauge_sensor_level() may be called because the sensor is triggered again. We now have to remove the pending clear request because the sensor should be activated (see [3]). However, since SensorInfo::clear() does not check the value of '_pending_clear_count' again, it still clears the sensor. The sensor may then be triggered again although the threshold value was only crossed once. As a result, the output of getUsageThresholdCount() is higher than expected.
> 
> Here is the detailed trace of events (the info in brackets is the output of SensorInfo::print):
> 
> ...
> 
> [on count = 1 pending_triggers = 0 pending_clears = 0]
> 
>  WB.freeCodeBlob()
>    LowMemoryDetector::detect_low_memory()
>      SensorInfo::set_gauge_sensor_level()
>        is_below_low ->
>          _pending_clear_count++;
> 
> [on count = 1 pending_triggers = 0 pending_clears = 1]
> 
>  WB.allocateCodeBlob()
>    LowMemoryDetector::detect_low_memory()
>      SensorInfo::set_gauge_sensor_level()
>        is_over_high -> 
>          _pending_trigger_count++;
>          _pending_clear_count = 0;  <-- remove pending clear request because the sensor is triggered again
> 
> [on count = 1 pending_triggers = 1 pending_clears = 0]
>   
>  LowMemoryDetector::process_sensor_changes()
>    SensorInfo::process_pending_requests()  <-- still sees the old _pending_clear_count == 1 because there is a race
>      SensorInfo::clear()
> 
> [off count = 2 pending_triggers = 0 pending_clears = 0]
> 
>  WB.fullGC()
>    LowMemoryDetector::detect_low_memory()
>      SensorInfo::set_gauge_sensor_level()
>        is_over_high -> _pending_trigger_count++;
> 
> [off count = 2 pending_triggers = 1 pending_clears = 0]
> 
> At this point the threshold count should be 2 because the threshold was only hit once. However, we have a pending trigger request. Now there are tow possibilities:
> 1) We check the counter before the pending trigger is processed: We continue because as expected the count is 2 but we fail later because the overall count does not match the expected value (RuntimeException: Unexpected threshold usage count (assert failed: 11 == 10)).
> 2) We check after the pending trigger is processed: The counter is 3 and we time out waiting for it to be 2.
> 
> Solution:
> I added asserts to the code to make sure that such race conditions are detected. I changed the implementation of SensorInfo::clear() to acquire the Service_lock and bail out if _pending_clear_count was reset in the meantime (i.e., if we lost a race to SensorInfo::set_gauge_sensor_level()). I also added a missing 
> 
>  335     _sensor_count += count;
> 
> to SensorInfo::clear() because it may trigger counter increments as well.
> 
> Testing:
> - 15k runs of failing testcase
> - JPRT
> 
> Thanks,
> Tobias
> 
> 
> [1] http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/da1c9ea76ce5/test/compiler/codecache/jmx/UsageThresholdExceededTest.java
> [2] http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/da1c9ea76ce5/test/compiler/codecache/jmx/CodeCacheUtils.java#l50
> [3] http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/da1c9ea76ce5/src/share/vm/services/lowMemoryDetector.cpp#l222
>