RFR: JDK-8255978: [windows] os::release_memory may not release the full range [v3]

Thomas Stuefe stuefe at openjdk.java.net
Tue Nov 17 07:15:11 UTC 2020


> On Windows, os::release_memory(p,size) may not actually release the whole region if it contains multiple mappings. This may cause memory bloat or runaway leaks or errors which look like failed mappings to specific wish addresses.
> 
> Background:
> 
> On Windows, memory mappings are established with VirtualAlloc() [1] and released with VirtualFree() [2]. In constrast to POSIX munmap(), VirtualFree() can only release a single full range established with VirtualAlloc(). It cannot release multiple ranges, or parts of a range.
> 
> The Windows implementation of os::release_memory(p, size) [3] calls VirtualFree(p, NULL, MEM_RELEASE) - it ignores the size parameter and releases whatever mapping happens to start at p:
> 
> bool os::pd_release_memory(char* addr, size_t bytes) {
>   return VirtualFree(addr, 0, MEM_RELEASE) != 0;
> }
> 
> ... which assumes that the given range size corresponds to the size of a mapping starting at p.
> 
> This may be incorrect:
> 
> 1) For NUMA-friendly allocation, we allocate memory in stripes, each stripe individually allocated.
> 2) For +UseLargePagesIndividualAllocation we do the same
> 3) apart from that, the given region size may just be wrong. Since we never check these, we may never have noticed. I am currently running tests to find out if we have other mismatched releases.
> 
> For cases (1) and (2), we would just release the first stripe in that striped range, leaving the rest of the mappings intact. This is not immediately noticeable, since VirtualFree() returns success. And even if it did not, we usually ignore the return code of os::release_memory().
> 
> The problem is aggrevated since, on Windows, we often employ an "optimistically-release-and-remap" approach: since mappings are undivisible, if one wants to change their size, split them or similar, one has to follow this sequence:
> 
> a) release old allocation
> b) place into the now vacated address room one or more new allocations
> 
> This is not guaranteed to work, since between (a) and (b) someone may have grabbed the address space. We live with that since there is no way to do this differently.
> 
> When used on a range which contains multiple mappings, this technique is almost guaranteed to fail. In that case, (a) would only release the first mapping in the range. (b) would almost certainly fail since most of the original range would still be mapped.
> 
> Examples of these technique in os_windows.cpp:
> - os::split_reseved_memory() (see also [4])
> - map_or_reserve_memory_aligned()
> - os::replace_existing_mapping_with_file_mapping()
> 
> This can manifest as small memory leak or inability to attach to a given wish address. It could also result in a viscous loop ([5], [6]) and result in ballooning and native OOMs.
> 
> --
> 
> AFAICS this is an old issue, dating back to at least jdk 8.
> 
> --
> 
> The change in detail:
> 
> - os::release_memory() on Window now:
> 	- in LP/NUMA case we unmap all mappings in the given range. The mappings must match the range exactly, otherwise we assert.
> 	- In normal case we just check the given range and assert if it does not match.
> 	Note: in release builds I just print a warning and return false. Should this be a guarantee? I leave that up to the Reviewers.
> 
> - Added gtests which test os::release_memory() with a variety of scenarios. Includes both positive and negative tests. The latter includes a death test for debug.
> 
> - Added a helper function to find mappings, for now windows only (os::win32::find_mapping()), including gtests for that function.
> 
> - Added a function to print mappings, for diagnostic purposes: os::print_memory_mappings(). Provided an implementation for linux and windows.
> 
> 
> --
> [1] https://docs.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-virtualalloc
> [2] https://docs.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-virtualfree
> [3] https://github.com/openjdk/jdk/blob/5dfb42fc68099278cbc98e25fb8a91fd957c12e2/src/hotspot/os/windows/os_windows.cpp#L3394
> [4] https://bugs.openjdk.java.net/browse/JDK-8253649
> [5] https://github.com/openjdk/jdk/blob/5dfb42fc68099278cbc98e25fb8a91fd957c12e2/src/hotspot/os/windows/os_windows.cpp#L3150
> [6] https://bugs.openjdk.java.net/browse/JDK-8255954

Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits:

 - Merge
 - Merge
 - Initial

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

Changes: https://git.openjdk.java.net/jdk/pull/1143/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1143&range=02
  Stats: 491 lines in 7 files changed: 490 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1143.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1143/head:pull/1143

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


More information about the hotspot-runtime-dev mailing list