RFR: 8357086: os::xxx functions returning memory size should return size_t [v13]

[Anton Artemov]

On Thu, 31 Jul 2025 09:43:18 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> Right, I looked at another place, sorry.
>> 
>> In `total_swap_space` one wants to compute the total swap space out of two values, memory_limit and swap_limit, by the following formula: 
>> 
>> total_swap_space = (memory_limit  + swap_limit) - memory_limit
>> 
>> the 1st summand (in braces) is returned by  `OSContainer::memory_and_swap_limit_in_bytes()`, the 2nd by `OSContainer::memory_limit_in_bytes()`. The problem is that `swap_limit` can be unbounded (represented by -1 in the container code). So we end up in returning `static_cast<size_t>(-1)`, which would be 2^64-1 on 64bit platform and is the same as `std::numeric_limits<size_t>::max()`.
>> 
>> In my opinion, it is a good representation of the _unbounded_ value.
>> 
>> There are only 2 places where `os::total_swap_space()` is used, both in JFR-related code. In both cases, the `size_t` values obtained from `os::total_swap_space()` is converted to a signed 64bit value (either jlong or s8, which is the same) with `static_cast`. If `size_t` has the same bitness as `jlong`, then one will get back -1 after conversion, so nothing is changed from the JFR perspective. However, if `size_t` is 32bit, then such conversion will not return -1, but instead a positive value of 2^32-1. It has to be addressed separately.
>
>> the 1st summand (in braces) is returned by `OSContainer::memory_and_swap_limit_in_bytes()`, the 2nd by `OSContainer::memory_limit_in_bytes()`. The problem is that `swap_limit` can be unbounded (represented by -1 in the container code). So we end up in returning `static_cast<size_t>(-1)`, which would be 2^64-1 on 64bit platform and is the same as `std::numeric_limits<size_t>::max()`.
>> 
>> In my opinion, it is a good representation of the _unbounded_ value.
> 
> The error code -1 happens to be interpretable as a large number if it is converted to a unsigned value, and one can argue that this large number is an OK value to represent an unbound value. Personally, I think that is more accidental than the intended behavior of this code. This is an obscure detail in the code, and given that it isn't immediately obvious for the reader it also makes it a risk that people will not understand this if they make future changes to the code.
> 
> And even we accept that -1 can be used as the unbound value, then I think it is wrong to use the unbound value in this code. If the container says that the swap space is unbound can't the underlying host still return a sensible value in si.totalswap? If this is the case, then an error in the container code will cause os::total_swap_space to return a way higher swap value than what the host is configured to have. 
> 
> With all this said, I think the above code should check if memory_and_swap_limit_in_bytes returns -1, just like we do in os::free_swap_space().

I modified the behavior of that method as suggested, it now reports the host total swap if the container returned -1.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25450#discussion_r2245599200