RFR: JDK-8280940: gtest os.release_multi_mappings_vm is racy [v2]

Daniel D.Daugherty dcubed at openjdk.java.net
Fri Feb 4 16:16:09 UTC 2022


On Fri, 4 Feb 2022 05:32:50 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> release_multi_mappings_vm is unfortunately racy.
>> 
>> The original intention of the test was to check that `os::release_memory()` works across multiple mappings allocated with multiple calls to `os::reserve_memory()`. This was broken for a long time on windows, since it relies on the implicit assumption that every platform uses mmap-ish APIs under the hood. But Windows virtual memory API (and SysV shmat, for that matter) does not work that way.
>> 
>> The release_multi_mappings_vm test 
>> - A reserves a number of mappings in 4M stripes adjacent to each other 
>> - B releases them with a single call to os::release_memory
>> - C re-allocates a range at the same address
>> 
>> Step (C) will fail if the os::release call in (B) failed to release the mapping. Which it sometimes did silently, so just checking the return code in (B) was not sufficient.
>> 
>> Unfortunately, it will also fail if someone concurrently mapped into the range between (B) and (C). It's rare, but it happens.
>> 
>> This is difficult to make completely airtight, but we could make it much more stable:
>> 1) instead of releasing all stripes, just release the n middle stripes (n>1) and leave first and last stripe reserved. Then, in (C), re-reserve the middle stripes. Tests the same (as long as we have multiple middle stripes) but drastically decreases the chance of some random allocation placing memory into the vacated address hole
>> 2) reduce the stripe size from today's 4M to something much smaller. Again, reduces the chance of stray mappings being placed into the hole since it will be smaller too.
>> 
>> The patch does just that.
>> It also adds a lengthy comment.
>> I also needed to place the NUMASwitcher line in front of the last os::release_memory. That is done just  to clean up everything before the tests ends, but now its a multi-mapping-release too (we now release front stripe, middle, and end stripe).
>> 
>> Tests:
>> - GHAs
>> - manually ran the test on x64 Linux
>> - SAP nightlies
>
> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Dans remarks

Thumbs up.

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

Marked as reviewed by dcubed (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7288


More information about the hotspot-runtime-dev mailing list