RFR: JDK-8298445: Add LeakSanitizer support in HotSpot [v2]

[Thomas Stuefe]

On Thu, 2 Feb 2023 06:34:56 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Justin King has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Support CDS
>>   
>>   Signed-off-by: Justin King <jcking at google.com>
>
> Thinking this through some more:
> 
> Why would UseCompressedClassPointers cause leak analysis to fail? Because you track Metaspace? If so, we should think whether it makes sense to track Metaspace.
> 
> Metaspace memory is tied to class loaders. Class loaders may live forever, until VM death, in that case Metaspace will not be released. How exactly does the leak profiler deal with that? Does it report those cases as leaks?
> 
> Metaspace "leaks" exist, but these are typically class loader leaks at the application level, and those you cannot recognize from the JVM. You need application knowledge for that.

> @tstuefe
> 
> Okay, I removed the forceful disabling of `UseCompressedOops` and `UseCompressedClassPointers` and it looks clean, so I removed that. I think what happened was two things. First, when I started this many months ago I did not have a full understanding of the different memory regions used by Hotspot compared to now and second when I was originally doing this I got false positive feedback by disabling these likely due to missing registration of a root region. So I originally chalked it up to being due to misaligned pointers. My original goal was to just get a clean run, so I took the hammer approach.
> 
> As long as the pointers to malloc-based memory are properly aligned in one of the root regions (ones we register or other memory allocated by malloc), LSan is happy. It shouldn't matter whether pointers to data in metaspace are compressed or not, or oops. Both reside in metaspace and java heap, neither of which LSan is directly concerned about other than looking for pointers to malloc-based memory. So you are correct in that regard that neither `UseCompressedOops` and `UseCompressedClassPointers` should be necessary. However metaspace leaks do exist and this setup actually found one (JDK-8298084), so metaspace being instrumented is definitely useful.

Thanks for the explanation. Do I understand correctly: You register address ranges with LSAN, and at certain points (program exit?) the memory is examined for pointers that point still to unreleased memory? Does LSAN instrument malloc and free?

> 
> To clarify, LSan is only concerned with leaks related to memory obtained from malloc and friends. It does not deal with metaspace leaks directly or java heap leaks.

Metaspace objects hold pointers to malloced memory. Metaspace Objects can leak, that's accepted. So these pointers would leak too. Would that not give us tons of false positives?

In other words, in Metaspace, we need to distinguish real malloc pointer leaks from allowed ones, otherwise monitoring Metaspace is not useful. Real leaks would be malloc pointers in metaspace objects that had been implicitly released by metaspace arena death.

I need to think more about this some more. Tracking leaks from metaspace objects could be useful. We could always postpone tracking Metaspace malloc pointers to another RFE, in which case I would remove the changes to VirtualSpaceNode.

@dholmes-ora : Do all the sub-data tied to metaspace objects really have to live in C-heap? They mostly do have the same lifetime as the parent object. It would be logical for them to live in Metaspace too - no need to explicitly release them in the various release_C_heap_structures. Cheaper too, since malloc is comparatively expensive.

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

PR: https://git.openjdk.org/jdk/pull/12229