RFR: 8340631: assert(reserved_rgn->contain_region(base_addr, size)) failed: Reserved CDS region should contain this mapping region [v6]

[Ioi Lam]

On Thu, 19 Dec 2024 17:27:13 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> What testing with `UseCompressedClassPointers` set to false, this assert is hit due to memory being released twice during the failure path of CDS archive mapping. This patch makes it so the RW and RO regions are only released once at the end in `MetaspaceShared::release_reserved_spaces()`. 
>> 
>> This patch hides `unmap_region` as the method should not be called on regions that were reserved. Instead, the region is skipped and we verify that it is indeed in the reserved space. Verified` with tier 1-4 tests.
>
> Matias Saavedra Silva has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Corrected edge case

> I would like to propose that we push the assert into unmap_region so that we don't have to duplicate the logic to decide if we should assert or unmap. Please take a look at this version of the fix and see what you think about it: [master...stefank:jdk:pull/22743](https://github.com/openjdk/jdk/compare/master...stefank:jdk:pull/22743)

I think this is better.

Stefan's change also reveals a new bug -- `unmap_stand_alone_region(MetaspaceShared::hp)` shouldn't be called by `FileMapInfo::map_heap_region_impl()`, as it will unreserve a block of memory inside the reserved heap region. This call should be removed.

This failure should be very very rare: The bitmap region is very small. If the process can't even map that, it's on the verge of dying due to OOM anyway. That's why we haven't caught this in the wild, but we should fix it nonetheless.

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

PR Comment: https://git.openjdk.org/jdk/pull/22743#issuecomment-2567337535