RFR: 8332506: SIGFPE In ObjectSynchronizer::is_async_deflation_needed() [v2]

[Coleen Phillimore]

On Thu, 2 Jan 2025 14:43:22 GMT, Fredrik Bredberg <fbredberg at openjdk.org> wrote:

>> This PR solves a division by zero problem, that according to the bug report happened in `ObjectSynchronizer::is_async_deflation_needed()`. As it turns out it really happened in the inlined  `monitors_used_above_threshold()` function. The problematic line looked like this:
>> 
>> `size_t monitor_usage = (monitors_used * 100LL) / ceiling;`
>> 
>> Unfortunately the `ceiling` value was increased every time there where too many deflations without any progress. This whould eventually lead to an overflow in the `ceiling` value, and in unlucky circumstances, it would become zero. Thus causing a division by zero crash.
>> 
>> This PR makes sure not to increase the `ceiling` value if `monitor_usage` is below the `MonitorUsedDeflationThreshold`.
>> It also makes sure the ceiling value is never zero, and does not wrap around.
>> 
>> Tested okay in tier1-5 and `test/hotspot/jtreg/runtime/Monitor/MonitorUsedDeflationThresholdTest.java`
>
> Fredrik Bredberg has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update after review

I walked through the math and this looks good.  This solves both problems of the ceiling being incremented too often and the overflow problem because of the first problem.

src/hotspot/share/runtime/synchronizer.cpp line 1279:

> 1277:   size_t monitor_usage = (monitors_used * 100LL) / ceiling;
> 1278:   if (int(monitor_usage) > MonitorUsedDeflationThreshold) {
> 1279:     bool status = true;

Above this, can you add a comment that's something like:
// Deflate monitors if over the threshold percentage unless no progress on previous deflations

Maybe rename 'status' to be 'is_above_threshold' would be more meaningful.

-------------

Marked as reviewed by coleenp (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22815#pullrequestreview-2529539650
PR Review Comment: https://git.openjdk.org/jdk/pull/22815#discussion_r1902058329