RFR: JDK-8327169: serviceability/dcmd/vm/SystemMapTest.java and SystemDumpMapTest.java may fail after JDK-8326586

[Matthias Baesken]

On Wed, 20 Mar 2024 08:28:45 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

> https://bugs.openjdk.org/browse/JDK-8326586 improved speed of System.map by exploiting the fact that VMA addresses are usually ordered ascending when read from /proc/pid/maps, and keeping track of the index of the last matching NMT region. That was a dramatic speed increase.
> 
> However, there is an embarrassing error. The index was kept inside a function-local static variable. But it depends on the number of NMT regions, and after calling System.map once at point-in-time A, it is equal to (number of NMT regions that were alive at A). If we call System.map again, at point-in-time B, and in the meantime mappings were released and we have fewer NMT regions, the stored index refers to an invalid position (larger than NMT region array count).
> 
> This messes up the next System.map printing. It can also theoretically cause a crash, since we dereference that out-of-bounds index, but I was not able to reproduce (the referenced array is usually over-allocated). Since we use the wrong - too high - index, we never enter the printing loop inside `CachedNMTInformation::lookup`, so we never print NMT regions. This makes the tests fail.
> 
> Tests: I was able to intermittendly reproduce the error without the patch by running a JVM and simulating NMT region cleanup between two calls to System.map. With this patch, the error is gone.
> 
> --
> 
> The obvious solution is to not use function-scope statics, but to store the last used index in a member inside CachedNMTInformation, and thus give it the same lifetime as that cache.

Marked as reviewed by mbaesken (Reviewer).

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

PR Review: https://git.openjdk.org/jdk/pull/18392#pullrequestreview-1948448239