RFR: JDK-8280940: gtest os.release_multi_mappings_vm is racy

[Daniel D.Daugherty]

On Mon, 31 Jan 2022 12:20:58 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 NUMASwitchter line to the front of the function since now the last os::release_memory - done to clean up everything before the tests ends - is 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

Thumbs up.

You may want to correct 'NUMASwitchter' in the PR so that
any searches find the correct name.

test/hotspot/gtest/runtime/test_os.cpp line 490:

> 488:   PRINT_MAPPINGS("B");
> 489: 
> 490:   // ...re-reserve the middle stripes. This should work unless release failed.

Perhaps:
    s/failed/failed silently/

just to be clear.

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

Marked as reviewed by dcubed (Reviewer).

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