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

[Thomas Stuefe]

On Wed, 20 Mar 2024 10:13:18 GMT, Matthias Baesken <mbaesken 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).

Thanks @MBaesken and @jdksjolen .

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

PR Comment: https://git.openjdk.org/jdk/pull/18392#issuecomment-2012114525